You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@gmail.com> on 2009/07/01 14:31:50 UTC
Re: svn commit: r789965 - in /apr/apr/trunk: CHANGES
dbd/apr_dbd_pgsql.c
2009/6/30 <bo...@apache.org>
> Author: bojan
> Date: Tue Jun 30 21:35:27 2009
> New Revision: 789965
>
> URL: http://svn.apache.org/viewvc?rev=789965&view=rev
> Log:
> Use locally scoped variables to avoid stomping on return codes.
> PR 47431.
> Patch by Wayne Jensen <wayne_jensen trendmicro.com>.
IMO the better way to handle this would have been
int ret, tmpret;
and using tmpret when the function-wide ret shouldn't be touched
Overloading "ret" and the semi-hidden setting of the function-wide ret make
this code less clear than it could be.
Continuing down the overly picky trail: It would be better to focus CHANGES
entries on the impact to library consumers, and omit details like
"locally[-]scoped variables".
Re: svn commit: r789965 - in /apr/apr/trunk: CHANGES
dbd/apr_dbd_pgsql.c
Posted by Bojan Smojver <bo...@rexursive.com>.
On Wed, 2009-07-01 at 20:35 -0400, Jeff Trawick wrote:
> not a -1; just something to think about in the future
OK, thanks.
--
Bojan
Re: svn commit: r789965 - in /apr/apr/trunk: CHANGES
dbd/apr_dbd_pgsql.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Jul 1, 2009 at 5:38 PM, Bojan Smojver <bo...@rexursive.com> wrote:
> On Wed, 2009-07-01 at 08:31 -0400, Jeff Trawick wrote:
> > IMO the better way to handle this would have been
> >
> > int ret, tmpret;
> >
> > and using tmpret when the function-wide ret shouldn't be touched
> >
> > Overloading "ret" and the semi-hidden setting of the function-wide ret
> > make this code less clear than it could be.
>
> That is what the original patch had, in fact. I kinda liked the locally
> scoped approach better, because it seemed cleaner to me. But then again,
> I'm not known for very good taste.
>
> > Continuing down the overly picky trail: It would be better to focus
> > CHANGES entries on the impact to library consumers, and omit details
> > like "locally[-]scoped variables".
>
> OK.
>
> Question: was your comment essentially a -1, in which case I'll
> revert/change, or was it just a remark?
not a -1; just something to think about in the future
Re: svn commit: r789965 - in /apr/apr/trunk: CHANGES
dbd/apr_dbd_pgsql.c
Posted by Bojan Smojver <bo...@rexursive.com>.
On Wed, 2009-07-01 at 08:31 -0400, Jeff Trawick wrote:
> IMO the better way to handle this would have been
>
> int ret, tmpret;
>
> and using tmpret when the function-wide ret shouldn't be touched
>
> Overloading "ret" and the semi-hidden setting of the function-wide ret
> make this code less clear than it could be.
That is what the original patch had, in fact. I kinda liked the locally
scoped approach better, because it seemed cleaner to me. But then again,
I'm not known for very good taste.
> Continuing down the overly picky trail: It would be better to focus
> CHANGES entries on the impact to library consumers, and omit details
> like "locally[-]scoped variables".
OK.
Question: was your comment essentially a -1, in which case I'll
revert/change, or was it just a remark?
--
Bojan