[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: nstrftime.c fails to build due to memset overflow
From: |
Marcus Müller |
Subject: |
Re: nstrftime.c fails to build due to memset overflow |
Date: |
Thu, 16 Mar 2023 10:29:43 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
Hi Bruno,
On 16.03.23 04:46, Bruno Haible wrote:
Apparently Paul and you have different ways of reading code, which
leads to different measures of "readability". You two could keep
contradicting each other eternally; that's not fruitful.
I don't intend to do that :) Paul seems to be the more experienced programmer than me, and
I wouldn't see the value in any kind of flamewar, either. Still was kind of uneasy seeing
the suggested code and not saying something – here we are!
I consider code paths that intentionally differ between debug and release
builds detrimental to code quality
This is a valid argument. But read the code that Paul proposed: It does
not conditionalize on NDEBUG. It conditionalizes on GCC_LINT || lint.
These macros can be turned on independently of NDEBUG.
That is an undisputable fact; but my problem here was not the technical impossibility, but
the ways that comes back to bite someone. See, for example canonicalize-lgpl.c, where
GCC_LINT gets set conditioned on _LIBC being defined. That is kind of surprising, isn't
it? That's the "introduction of conditionality where clarity would be preferred" I was
referring to. Sure, this isn't canonicalize-lgpl.c, so that setting there has no effect
here, but if I have a codebase that does such things, I'm bound to be checking every
single expansion for preconditions in all the internal headers I include. That's just hard
to work with, for me.
Generally, I would (my perspective) try to minimized the moment of surprise; a =0 to me is
less surprising. I know Paul sees this differently, but I'm not arguing against him being
right with how he reads the code.
I'm arguing that introduction of separate code paths is the hard-to-justify thing. (But
Paul already replied that he wouldn't want the macro either, so, not even sure we disagree
on that. I think we both want the code to be as unmodified as possible. I still would want
to silence the warning, because the *class* of warnings has proven to be very productive,
so disabling it, or having false positives in there, is kind of making code improvement
harder – but, I feel it's fair to stress that – this is my perspective on things. I mean,
gnulib CHANGELOG tells me that there's a few things that coverity and other tools might
have prevented.)
There's a big difference between macro-loaded code, as in e.g.
https://gitlab.inria.fr/gustedt/p99/-/tree/master/p99
or https://github.com/LeoVen/C-Macro-Collections ,
and a simple UNINIT macro that Paul proposed, that does not even
hamper debugging with gdb.
a) Wow, didn't know these!
b) No doubt. Still, and I think Paul's last two emails explicitly agreed here, it's better
to *not* have the macro than to have it.
The original white_add macro which gcc falsely finds a -1 in,
which kicked off this thread? That should really not be part
of any code base.
GCC would give the same warning if you would pass it the macro-
expanded source code ("gcc -E" output).
I know, I had to manually copy and paste half-expanded code into the place where it's
called, just to be sure I'm not missing something – due to the non-locality of what the
macro uses. Because I honestly thought that it's more likely I can't read the code,
multi-layered and convoluted and non-explicit-variable depending as it is, than that the
compiler cries wolf in the presence of naught. Thankfully, I've been proven wrong! That's
good on multiple levels: Neither is the code dysfunctional, nor am I going insane.
Therefore it is irrelevant
whether white_add is a macro, a function, or entirely inlined.
Seeing that you start this email on a personal note (thanks!) to reduce the chance of
prolonged exchange of contradictory perspectives without a path to actually producing a
positive outcome, I'm not quite sure how great it'd be of me to contradict you here:
To me, it wasn't irrelevant.
If this macro wasn't so bad, looking at it anyone could have instantly deduced that this
is a compiler diagnostic in error, and went on to act on that knowledge. I wrote that "bug
report" email because I felt I couldn't fix this code and must be missing something.
Best regards, and again, thanks for putting up with my perspectives,
Marcus