coreutils
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]