[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils
Subject:    Re: nstrftime.c fails to build due to memset overflow
From:       Marcus_Müller <marcus () hostalia ! de>
Date:       2023-03-16 9:29:43
Message-ID: 735280e3-c856-a747-dc16-dc79c1a1f2b2 () hostalia ! de
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]