You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Russell Yanofsky <re...@columbia.edu> on 2003/06/27 08:09:35 UTC

[PATCH] python bindings bugfix for svn_repos_get_logs()

Log Message:

Bugfix for the python bindings. svn_repos_get_logs() causes a crash when
its "discover_changed_paths" argument is nonzero because the thunk,
svn_swig_py_thunk_log_receiver(), that forwards to a user defined python
callback function is unable to find a swig type corresponding
to the svn_log_changed_path_t structs that it has to pass to the callback.

* subversion/bindings/swig/swigutil_py.c (svn_swig_py_thunk_log_receiver):
  changed argument to SWIG_TypeQuery for "svn_log_changed_path_t" type

* subversion/bindings/swig/core.i: exposed "svn_log_changed_path_t" type


begin 666 changed_paths.diff
M26YD97@Z('-U8G9E<G-I;VXO8FEN9&EN9W,O<W=I9R]C;W)E+FD*/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/0HM+2T@<W5B=F5R<VEO;B]B:6YD:6YG<R]S=VEG
M+V-O<F4N:0DH<F5V:7-I;VX@-C,R.2D**RLK('-U8G9E<G-I;VXO8FEN9&EN
M9W,O<W=I9R]C;W)E+FD)*'=O<FMI;F<@8V]P>2D*0$ @+3,S+#<@*S,S+#8@
M0$ *( H@+RH@(R,C(&9O<B!N;W<L(&QE="=S(&YO="!T<GD@=&\@:&%N9&QE
M('1H97-E('-T<G5C='5R97,N('-W:6<@8V]M<&QA:6YS"B @(" C(R,@86)O
M=70@<V5T=&EN9R!T:&4@)V-O;G-T(&-H87(@*B<@:6YS:61E('1H92!S=')U
M8W0@;6EG:'0@;&5A:R!M96T@("HO"BTE:6=N;W)E('-V;E]L;V=?8VAA;F=E
M9%]P871H7W0["B *("\J(",C(R!792!A;'-O(&=E="!C;VUP;&%I;G1S(&%B
M;W5T('!O<W-I8FQE(&UE;6]R>2!L96%K86=E(&9O<B!S=FY?9&ER96YT+ H@
M(" @(R,C(&)U="!W92!C86X@;&EV92!W:71H(&ET(&9O<B!N;W<N("HO"D! 
M("TT,BPV("LT,2PQ,R! 0 H@+RH@(R,C(&9O<B!N;W<L(&QE="=S(&EG;F]R
M92!T:&ES('1H:6YG+B J+PH@)6EG;F]R92!S=FY?<')O<%]T.PH@"BLO*B M
M+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM
M+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+0HK(" @5&AE(")C;W!Y9G)O;5]P
M871H(B!M96UB97(@;V8@82 B<W9N7VQO9U]C:&%N9V5D7W!A=&A?="(@<W1R
M=6-T(&AA<R!T;PHK(" @8F4@<F5A9"!O;FQY(&)E8V%U<V4@=&AE<F4G<R!N
M;R!R96QI86)L92!W87D@=&\@9G)E92!T:&4@;VQD('9A;'5E"BL@("!W:&5N
M(&%S<VEG;FEN9R!T:&4@;F5W(&]N92X@"BLJ+PHK)6EM;75T86)L92!S=FY?
M;&]G7V-H86YG961?<&%T:%]T.CIC;W!Y9G)O;5]P871H.PHK"B E:6YC;'5D
M92!S=FY?='EP97,N: H@"B *26YD97@Z('-U8G9E<G-I;VXO8FEN9&EN9W,O
M<W=I9R]S=VEG=71I;%]P>2YC"CT]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]
M/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T]/3T*+2TM
M('-U8G9E<G-I;VXO8FEN9&EN9W,O<W=I9R]S=VEG=71I;%]P>2YC"2AR979I
M<VEO;B V,S(Y*0HK*RL@<W5B=F5R<VEO;B]B:6YD:6YG<R]S=VEG+W-W:6=U
M=&EL7W!Y+F,)*'=O<FMI;F<@8V]P>2D*0$ @+3$P,S@L-R K,3 S."PW($! 
M"B!["B @(%!Y3V)J96-T("IR96-E:79E<B ](&)A=&]N.PH@("!0>4]B:F5C
M=" J<F5S=6QT.PHM("!S=VEG7W1Y<&5?:6YF;R J=&EN9F\@/2!35TE'7U1Y
M<&51=65R>2@B4U=)1U194$5?<%]S=FY?;&]G7V-H86YG961?<&%T:%]T(BD[
M"BL@('-W:6=?='EP95]I;F9O("IT:6YF;R ](%-724=?5'EP95%U97)Y*")S
M=FY?;&]G7V-H86YG961?<&%T:%]T("HB*3L*(" @4'E/8FIE8W0@*F-H<&%T
<:',["B @('-V;E]E<G)O<E]T("IE<G(["B @"@``
`
end



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

Re: [PATCH] python bindings bugfix for svn_repos_get_logs (resend, issue 1387)

Posted by Russell Yanofsky <re...@columbia.edu>.
cmpilato@collab.net wrote:
> "Russell Yanofsky" <re...@columbia.edu> writes:
>
>> The immutable marker just prevents users from assigning their own
>> values to that member so there can't be any leaks.
>
> Sounds great -- thanks for explaining.  Would you mind (if you have
> the time) giving the rest of our bindings code a once-over to see if
> we need such a restriction placed on other class members?

A patch is attached that places the same restriction on other const char *
members. I didn't look over the bindings code, just dealt with swig warning
messages.

Log Message:

Prevent users of swig bindings from assigning to const char * struct members so
there won't be any memory leaks.

* subversion/bindings/swig/core.i: made struct members which cause
"Warning(451): Setting const
   char * member may leak memory" immutable.



Re: [PATCH] python bindings bugfix for svn_repos_get_logs (resend, issue 1387)

Posted by cm...@collab.net.
"Russell Yanofsky" <re...@columbia.edu> writes:

> The immutable marker just prevents users from assigning their own
> values to that member so there can't be any leaks.

Sounds great -- thanks for explaining.  Would you mind (if you have
the time) giving the rest of our bindings code a once-over to see if
we need such a restriction placed on other class members?

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

Re: [PATCH] python bindings bugfix for svn_repos_get_logs (resend, issue 1387)

Posted by Russell Yanofsky <re...@columbia.edu>.
cmpilato@collab.net wrote:
> "Russell Yanofsky" <re...@columbia.edu> writes:
>
> > This is a trivial 3-line fix for the python bindings.
>
> Committed in revision 6579, though I'm not sure I understand
> the need for the 'immutable' marker. That string should be
> allocated from the pool, right? So clearing or destroying
> the pool would de-alloc the copyfrom_path. What am I
> mis-logic-ifying?

When subversion assigns a string to svn_log_changed_path_t::copyfrom_path, it
doesn't need to be freed since it is allocated in the pool. But if a python user
assigned a string to that member, the string would be allocated on the heap and
would need to be freed in order to prevent a memory leak. The immutable marker
just prevents users from assigning their own values to that member so there
can't be any leaks.

- Russ



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

Re: [PATCH] python bindings bugfix for svn_repos_get_logs (resend, issue 1387)

Posted by cm...@collab.net.
"Russell Yanofsky" <re...@columbia.edu> writes:

> This is a trivial 3-line fix for the python bindings. 

Committed in revision 6579, though I'm not sure I understand the need
for the 'immutable' marker.  That string should be allocated from the
pool, right?  So clearing or destroying the pool would de-alloc the
copyfrom_path.  What am I mis-logic-ifying?

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

[PATCH] python bindings bugfix for svn_repos_get_logs (resend, issue 1387)

Posted by Russell Yanofsky <re...@columbia.edu>.
This is a trivial 3-line fix for the python bindings. Several ViewCVS users have
reported errors caused by this bug:

  http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgId=270792
  http://mailman.lyra.org/pipermail/viewcvs/2003q3/002073.html
  http://mailman.lyra.org/pipermail/viewcvs-dev/2003-July/001082.html

And one user who applied the patch himself confirmed that it fixed the problem:

  http://mailman.lyra.org/pipermail/viewcvs/2003q3/002082.html

Can someone commit this?

- Russ


Log Message:

Bugfix for the python bindings. svn_repos_get_logs() causes a crash
when its "discover_changed_paths" argument is nonzero because the
thunk, svn_swig_py_thunk_log_receiver(), that forwards to a user
defined python callback function is unable to find a swig type
corresponding to the svn_log_changed_path_t structs that it has to
pass to the callback.

* subversion/bindings/swig/swigutil_py.c
  (svn_swig_py_thunk_log_receiver): changed argument to
  SWIG_TypeQuery for "svn_log_changed_path_t" type

* subversion/bindings/swig/core.i: exposed "svn_log_changed_path_t"
  type

Re: [PATCH] python bindings bugfix for svn_repos_get_logs()

Posted by Paul L Lussier <pl...@lanminds.com>.
In a message dated: Fri, 27 Jun 2003 04:09:35 EDT
"Russell Yanofsky" said:

>Log Message:
>
>Bugfix for the python bindings. svn_repos_get_logs() causes a crash when
>its "discover_changed_paths" argument is nonzero because the thunk,
>svn_swig_py_thunk_log_receiver(), that forwards to a user defined python
>callback function is unable to find a swig type corresponding
>to the svn_log_changed_path_t structs that it has to pass to the callback.

Filed as Issue 1387 here:
	http://subversion.tigris.org/issues/show_bug.cgi?id=1387

Tried to add Russell to the Cc: list in IZ, but he's evidently not a 
member...
-- 

Seeya,
Paul
--
Key fingerprint = 1660 FECC 5D21 D286 F853  E808 BB07 9239 53F1 28EE

	It may look like I'm just sitting here doing nothing,
   but I'm really actively waiting for all my problems to go away.

	 If you're not having fun, you're not doing it right!



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

Re: [PATCH] python bindings bugfix for svn_repos_get_logs()

Posted by Russell Yanofsky <re...@columbia.edu>.
Russell Yanofsky wrote:
>> Log Message:
>>
>> Bugfix for the python bindings. svn_repos_get_logs() causes a crash
>> when its "discover_changed_paths" argument is nonzero ...

I should add that an easy way to test this with ViewCVS installed is to edit the
vclib.svn.fetch_log() function to call:

    repos.svn_repos_get_logs(svnrepos.repos, [ full_name ],
                             svnrepos.rev, 0, 1, 1,
                             receiver.receive, svnrepos.pool)

instead of:

    repos.svn_repos_get_logs(svnrepos.repos, [ full_name ],
                             svnrepos.rev, 0, 0, 1,
                             receiver.receive, svnrepos.pool)

Without the patch, it will crash on the file log pages.

- Russ




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