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