gcc 4.2 warnings
At jsled's prompting, I'm taking a stab at hacking my way through gcc 4.2.1 (debian) warnings. And, at warlord's oh-so-kind prompting, I'm actually trying to do something about them. All credit for these will likely belong to whichever poor beleaguered dev happens to be on #gnucash at the moment. :) also note that these will be fixes that make the warning go away. Potential implications are a complete mystery to me... here's the first one. A -- Index: engine/TransLog.c === --- engine/TransLog.c (revision 16547) +++ engine/TransLog.c (working copy) @@ -273,7 +273,7 @@ gnc_numeric_denom(amt), gnc_numeric_num(val), gnc_numeric_denom(val), - drecn ? drecn : ""); + drecn[0] == 0 ? drecn : ""); } fprintf (trans_log, "= END\n"); signature.asc Description: Digital signature ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: gcc 4.2 warnings
Am Donnerstag, 4. Oktober 2007 22:56 schrieb Andrew Sackville-West: > At jsled's prompting, I'm taking a stab at hacking my way through gcc > 4.2.1 (debian) warnings. And, at warlord's oh-so-kind prompting, I'm > actually trying to do something about them. Patches are always great! Thanks for picking up this work on the newest gcc. However, in this particular case I think you didn't fix the warning the right way: > --- engine/TransLog.c (revision 16547) > +++ engine/TransLog.c (working copy) > @@ -273,7 +273,7 @@ > gnc_numeric_denom(amt), > gnc_numeric_num(val), > gnc_numeric_denom(val), > - drecn ? drecn : ""); > + drecn[0] == 0 ? drecn : ""); > } > > fprintf (trans_log, "= END\n"); The whole point of the expression (drecn ? drecn : "" ) is in long form (drecn != NULL ? drecn : ""), in other words, if the char pointer is non-NULL, use it, but otherwise use the pointer to an empty string. This is important for some OS which would crash if a NULL pointer is passed to printf() and friends, hence they should only get a (non-NULL) empty string. Your change, however, is a different expression: With your case, if the char pointer points to a non-empty string, use it, but otherwise use the pointer to an empty string. Which points me to the question: Which gcc warning did you try to fix here? Looking more into context, it turns out drecn is a local char buffer anyway, hence it can't be NULL anyway. Because of this, here (and only here) you should replace the expresssion direcly by drecn, i.e. - drecn ? drecn : ""); + drecn); Thanks a lot for starting to fix these warnings! Christian ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: gcc 4.2 warnings
Christian Stimming <[EMAIL PROTECTED]> writes:
> Your change, however, is a different expression: With your case, if the char
> pointer points to a non-empty string, use it, but otherwise use the pointer
> to an empty string.
>
> Which points me to the question: Which gcc warning did you try to fix here?
The warning was "TransLog.c:254: warning: the address of 'drecn' will always
evaluate as 'true'".
> Looking more into context, it turns out drecn is a local char buffer anyway,
> hence it can't be NULL anyway. Because of this, here (and only here) you
> should replace the expresssion direcly by drecn, i.e.
>
> - drecn ? drecn : "");
> + drecn);
I suggested the {{{ drecn[0] == 0 ? [...] }}} form of the fix.
Of course, "drecn[0] == 0" just means the string is already empty.
So, you're totally right; it should just be "drecn".
But, there was a bit of a miscommunication about how to make the change,
anyways. As you'd quoted, if drecn[0] == 0, then the string drecn is empty,
and it'll be printed. Otherwise (when drecn actually has a value), it will
use "" instead.
Thanks for the code review.
--
...jsled
http://asynchronous.org/ - a=jsled; b=asynchronous.org; echo [EMAIL PROTECTED]
pgp1LYZkHTop0.pgp
Description: PGP signature
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: gcc 4.2 warnings
On Fri, Oct 05, 2007 at 09:22:53PM +0200, Christian Stimming wrote: > Am Donnerstag, 4. Oktober 2007 22:56 schrieb Andrew Sackville-West: > > At jsled's prompting, I'm taking a stab at hacking my way through gcc > > 4.2.1 (debian) warnings. And, at warlord's oh-so-kind prompting, I'm > > actually trying to do something about them. > > Patches are always great! Thanks for picking up this work on the newest gcc. :). I don't know how much I can actually do but I'll poke away at it here and there. If gives my computer something to do while I try to grok the gnc:html-table-foo code to try and make it a little snappier (that's the itch I'm currently trying to scratch as it now takes me a good long while to run an income statement). > > However, in this particular case I think you didn't fix the warning the right > way: > > > --- engine/TransLog.c (revision 16547) > > +++ engine/TransLog.c (working copy) > > @@ -273,7 +273,7 @@ > > gnc_numeric_denom(amt), > > gnc_numeric_num(val), > > gnc_numeric_denom(val), > > - drecn ? drecn : ""); > > + drecn[0] == 0 ? drecn : ""); > > } > > > > fprintf (trans_log, "= END\n"); > > The whole point of the expression (drecn ? drecn : "" ) is in long form > (drecn != NULL ? drecn : ""), in other words, if the char pointer is > non-NULL, use it, but otherwise use the pointer to an empty string. This is > important for some OS which would crash if a NULL pointer is passed to > printf() and friends, hence they should only get a (non-NULL) empty string. > Your change, however, is a different expression: With your case, if the char > pointer points to a non-empty string, use it, but otherwise use the pointer > to an empty string. > > Which points me to the question: Which gcc warning did you try to fix here? jsled already answered this. THanks for the little lesson. I'll be posting more of them soon. > > Looking more into context, it turns out drecn is a local char buffer anyway, > hence it can't be NULL anyway. Because of this, here (and only here) you > should replace the expresssion direcly by drecn, i.e. > > - drecn ? drecn : ""); > + drecn); okay. I think I understand. drecn is a pointer and as such has the potential to be NULL which is very bad forthat one OS. But in this case, drecn is locally declared as a pointer to a character buffer and won't ever be NULL. hence, its okay to just pass it. right? > > Thanks a lot for starting to fix these warnings! :) A signature.asc Description: Digital signature ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: gcc 4.2 warnings
On Fri, Oct 05, 2007 at 05:57:01PM -0400, Josh Sled wrote:
> Christian Stimming <[EMAIL PROTECTED]> writes:
> > Your change, however, is a different expression: With your case, if the
> > char
> > pointer points to a non-empty string, use it, but otherwise use the pointer
> > to an empty string.
> >
> > Which points me to the question: Which gcc warning did you try to fix here?
>
> The warning was "TransLog.c:254: warning: the address of 'drecn' will always
> evaluate as 'true'".
>
>
> > Looking more into context, it turns out drecn is a local char buffer
> > anyway,
> > hence it can't be NULL anyway. Because of this, here (and only here) you
> > should replace the expresssion direcly by drecn, i.e.
> >
> > - drecn ? drecn : "");
> > + drecn);
>
> I suggested the {{{ drecn[0] == 0 ? [...] }}} form of the fix.
>
> Of course, "drecn[0] == 0" just means the string is already empty.
> So, you're totally right; it should just be "drecn".
>
> But, there was a bit of a miscommunication about how to make the change,
> anyways. As you'd quoted, if drecn[0] == 0, then the string drecn is empty,
> and it'll be printed. Otherwise (when drecn actually has a value), it will
> use "" instead.
whoops. that's not good!
A
signature.asc
Description: Digital signature
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
