Re: [Low priority] Some minor complaints from cppcheck about emacs 26.2

[Adam Richter]

[Sorry for the resend, Paul.  I forgot to cc emacs-devel.]

Thanks for the prompt detailed analysis, Paul.

I'm fine with whatever you want to do with update-game-score.c.  I
agree that exiting a few microseconds faster is more important than
appeasing a code checker in this case (not sarcasm).  If I had my
druthers, I'd probably put in a commented out free() so anyone else
running a similar check or trying to move the code to a library will
be less likely ask the same question, but whatever.  I'll look into
whether there is some comment that cppcheck will accept to indicate
that the leak is deliberate.

I see now that the stdint.h problems were substitutions from
stdint.in.h for HAVE_SIGNED_SIG_ATOMIC_T and two others (there were
apparently three such cases).  Trying another Linux environment just
now, my initial attempt to configure on an Ubuntu 19.04 system broke
and I have to run now and be away for the rest of the day, but I
suspect that you are right that it is my problem somewhere and hope to
send you an update confirming this in the next day or two.

Thanks for your explanation about nstrftime.c, which I read while
looking at the code.  I think you're right and am a bit surprised that
cppcheck did enough analysis to complain but not enough to rule it
out, given that cpp must have been using an ifdef combination that
would make the C preprocessor expand all of those macros.  I hope to
take a look at cppcheck side in the next day or two and will respond
further if I have some news.

I suspect that you're right in everything except for the
MATCH_MAY_ALLOCATE in regex.c.  If I run cppcheck
-DMATCH_MAY_ALLOCATE, it still generates the warning about the
compile_stack.stack.  I don't know that there is any urgency to fixing
it though, especially if you want to await a forthcoming code
simplification.

Adam

On Tue, Apr 30, 2019 at 3:23 PM Paul Eggert <address@hidden> wrote:
>
> On 4/30/19 2:38 PM, Adam Richter wrote:
> > lib-src/update-game-score.c <-- I submitted a patch attached to a
> > message to this mailing list a few minutes ago, which I think fixes
> > the legitimate complaints.  The last complaint ("realloc mistake")
> > does not appear to be a bug.
>
> Since update-game-score is about to exit when the 'free' calls you're
> proposing would be executed, I'm not sure the changes are that helpful:
> they complicate the source code and make the executable a tiny bit
> bigger and slower and don't fix any real leaks. Perhaps if we do full
> LeakSanitizer checking we can put in the change then, if only to pacify
> the LeakSanitizer. (It's not clear to me that LeakSanitizer is a win
> either, as it also reports quite a few false alarms.)
>
>
> > lib/stdint.h  <-- This is a real typographical bug and I don't know
> > what the author intended.
>
> There is no lib/stdint.h in the Emacs distribution. On some platforms
> lib/stdint.h is built from lib/stdint.in.h. If that build process didn't
> work for you, it sounds like you need need to look into what went wrong
> in that build process.
>
>
> >
> > lib/nstrftime.c <-- Possibly a real bug, but only if multibyte is
> > disabled.  A local variable "width", set to -1, is used by a macro
> > that I think may expect a positive value, but I am not sure of this
> > fix.
>
> I don't see any bug there; could you explain? If 'width' is negative
> then the macro's _w is zero, which means _n < _w cannot be true (both _n
> and _w are unsigned), and so the memset calls cannot be invoked.
>
> > nt/preprep.c
>
> Eli can look into that one if he has the time.
>
>
> > src/regex.c <--I think the tiny memory leak for compile_stack.stack is
> > real.
>
> Is this when MATCH_MAY_ALLOCATE is not defined? If so, let's not worry
> about it. It's always defined, and the !MATCH_MAY_ALLOCATE code has been
> removed in Emacs 27.
>