On 02/24/2012 01:12, Carlos R. Mafra wrote:
> On Fri, 24 Feb 2012 at 0:46:35 -0800, Doug Barton wrote:
>> On 02/23/2012 23:39, Paul Harris wrote:
>>> If that patch worked, then that looks like undefined behaviour.
>>
>> What triggered me to think about the change to logical negation turns
>> out to be what you could consider "a FreeBSD problem" after all. :) I
>> know that in some operating systems (and I believe all/most Linux'es?)
>> NULL is defined to be numeric 0, so equating them is safe to do. FreeBSD
>> doesn't do that, so it isn't safe. (Please note, I'm not trying to start
>> a "whose interpretation of the C standard is more correct" fight, I'm
>> just saying that FreeBSD does it differently than some/many others do.)
>>
>> Meanwhile, I tested that change with and without --dont-restore and it
>> worked as expected in both cases, so that change should go into the
>> repo.
>
> Which change?
The one I sent earlier today ... I attached it again.
> Can you send a properly "changeloged" patch? (ie a git
> patch with a commit message)
No, sorry, I don't use git. Here's the commit log that I used for the
change to the FreeBSD port:
Use logical negation instead of comparison to 0 for a value that can
be numeric, but is null by default.
>> On a more general note, if there are any other places where 'foo == 0'
>> is being used where logical negation is intended, especially when foo
>> can be null, they should also be fixed. A quick grep of the new code
>> (minus str*cmp of course) indicated too many places for me to trace the
>> code paths myself, sorry. But changing this would definitely make the
>> code more robust.
>
> Indeed, and using !foo even reads better.
>
> Is there some code checker which could point out those "violations"?
Paul's right, valgrind is almost certainly the right tool for that.
clang/llvm usually does a pretty good job of identifying obscure cases
like this, but it will be a while before I can get that set up as I'm
currently in the middle of some non-trivial work stuff. When I have time
I'll post a clang build log.
hth,
Doug
--
It's always a long day; 86400 doesn't fit into a short.
Breadth of IT experience, and depth of knowledge in the DNS.
Yours for the right price. :) http://SupersetSolutions.com/
--- src/startup.c.orig 2012-02-14 11:36:01.000000000 -0800
+++ src/startup.c 2012-02-23 15:50:19.000000000 -0800
@@ -761,7 +761,7 @@
wMenuRestoreState(wScreen[j]);
/* If we're not restarting, restore session */
- if (wPreferences.flags.restarting == 0 &&
!wPreferences.flags.norestore)
+ if (!wPreferences.flags.restarting &&
!wPreferences.flags.norestore)
wSessionRestoreState(wScreen[j]);
if (!wPreferences.flags.noautolaunch) {
