You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <B....@competence.biz> on 2008/04/10 14:03:09 UTC

Svn_wc_set_changelist should clear it's own error (memory leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)

	Hi,

svn_wc_set_changelist assumes al implementations of svn_wc_notify_func2
will clear the err value if it it's notifying its
svn_wc_notify_changelist_moved event.

In r27073 subversion/svn/notify.c was altered for this specific case.
But this requirement was not documented at the C api level; and I don't
think we can break the ABI so that the receiver of the notify (Any
subversion client, Tortoise, ...) is responsible for releasing the value
of a field in this specific case.

The releasing of the error should be moved back to the caller to remove
the api change since 1.4; or a pool cleanup function should be
registered as is the case in the svn_wc_dup_notify() method.

	Bert


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


RE: Svn_wc_set_changelist should clear it's own error (memory leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)

Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: donderdag 22 mei 2008 22:13
> To: Bert Huijben
> Cc: 'Ben Collins-Sussman'; dev@subversion.tigris.org
> Subject: Re: Svn_wc_set_changelist should clear it's own error (memory
> leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)
> 
> What became of this API-sensitive patch?

Applied to trunk by dlr in r30544, nominated for backport in r30545 and ported to 1.5.x in r30562 by markphip.

	Bert

> Bert Huijben wrote:
> >> -----Original Message-----
> >> From: sussman@gmail.com [mailto:sussman@gmail.com] On Behalf Of Ben
> >> Collins-Sussman
> >> Sent: donderdag 10 april 2008 17:29
> >> To: Bert Huijben
> >> Cc: dev@subversion.tigris.org
> >> Subject: Re: Svn_wc_set_changelist should clear it's own error
> (memory
> >> leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)
> >>
> >> Care to send a patch?
> >
> > [[
> > Removed unused svn_wc_notify_changelist_failed notification and moved
> the
> > release
> > of the err member passed to the notify function for
> > svn_wc_notify_changelist_moved
> > to the caller.
> >
> > This revision should partially be reverted if not merged to 1.5

<snip>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Svn_wc_set_changelist should clear it's own error (memory leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)

Posted by "C. Michael Pilato" <cm...@collab.net>.
What became of this API-sensitive patch?


Bert Huijben wrote:
>> -----Original Message-----
>> From: sussman@gmail.com [mailto:sussman@gmail.com] On Behalf Of Ben
>> Collins-Sussman
>> Sent: donderdag 10 april 2008 17:29
>> To: Bert Huijben
>> Cc: dev@subversion.tigris.org
>> Subject: Re: Svn_wc_set_changelist should clear it's own error (memory
>> leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)
>>
>> Care to send a patch?
> 
> [[
> Removed unused svn_wc_notify_changelist_failed notification and moved the
> release 
> of the err member passed to the notify function for
> svn_wc_notify_changelist_moved
> to the caller.
> 
> This revision should partially be reverted if not merged to 1.5
> 
> * subversion/include/svn_wc.h
>   (svn_wc_notify_action_t) Removed unused svn_wc_notify_changelist_failed
> 
> * subversion/libsvn_wc/adm_ops.c
>   (svn_wc_set_changelist) Release the error passed to the notification
> handler
> 
> * subversion/svn/notify.c
>   (notify) Removed handling of the svn_wc_notify_changelist_failed and stop
>   clearing the error passed to svn_wc_notify_changelist_moved.
> 
> * subversion/bindings/javahl/native/EnumMapper.cpp
>   (EnumMapper::mapNotifyAction) Removed mapping of
> svn_wc_notify_changelist_failed
> 
> *
> subversion/bindings/javahl/src/org/tigris/subversion/javahl/NotifyAction.jav
> a
>   (NotifyAction) Removed changelist_failed field and renumbered merge_begin
> 
> Patch by: Bert Huijben <b....@competence.biz>
> ]]
> 
> If the patch is not merged to 1.5, all changes except those in notify.c and 
> adm_ops.c must be reverted
> 
> 	Bert
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: Svn_wc_set_changelist should clear it's own error (memory leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)

Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: sussman@gmail.com [mailto:sussman@gmail.com] On Behalf Of Ben
> Collins-Sussman
> Sent: donderdag 10 april 2008 17:29
> To: Bert Huijben
> Cc: dev@subversion.tigris.org
> Subject: Re: Svn_wc_set_changelist should clear it's own error (memory
> leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)
> 
> Care to send a patch?

[[
Removed unused svn_wc_notify_changelist_failed notification and moved the
release 
of the err member passed to the notify function for
svn_wc_notify_changelist_moved
to the caller.

This revision should partially be reverted if not merged to 1.5

* subversion/include/svn_wc.h
  (svn_wc_notify_action_t) Removed unused svn_wc_notify_changelist_failed

* subversion/libsvn_wc/adm_ops.c
  (svn_wc_set_changelist) Release the error passed to the notification
handler

* subversion/svn/notify.c
  (notify) Removed handling of the svn_wc_notify_changelist_failed and stop
  clearing the error passed to svn_wc_notify_changelist_moved.

* subversion/bindings/javahl/native/EnumMapper.cpp
  (EnumMapper::mapNotifyAction) Removed mapping of
svn_wc_notify_changelist_failed

*
subversion/bindings/javahl/src/org/tigris/subversion/javahl/NotifyAction.jav
a
  (NotifyAction) Removed changelist_failed field and renumbered merge_begin

Patch by: Bert Huijben <b....@competence.biz>
]]

If the patch is not merged to 1.5, all changes except those in notify.c and 
adm_ops.c must be reverted

	Bert


Re: Svn_wc_set_changelist should clear it's own error (memory leak in +- all implementers of svn_wc_notify_func2) (Issue #3169)

Posted by Ben Collins-Sussman <su...@red-bean.com>.
Care to send a patch?

On Thu, Apr 10, 2008 at 9:03 AM, Bert Huijben <B....@competence.biz> wrote:
>         Hi,
>
>  svn_wc_set_changelist assumes al implementations of svn_wc_notify_func2
>  will clear the err value if it it's notifying its
>  svn_wc_notify_changelist_moved event.
>
>  In r27073 subversion/svn/notify.c was altered for this specific case.
>  But this requirement was not documented at the C api level; and I don't
>  think we can break the ABI so that the receiver of the notify (Any
>  subversion client, Tortoise, ...) is responsible for releasing the value
>  of a field in this specific case.
>
>  The releasing of the error should be moved back to the caller to remove
>  the api change since 1.4; or a pool cleanup function should be
>  registered as is the case in the svn_wc_dup_notify() method.
>
>         Bert
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>  For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org