unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: "Óscar Fuentes" <ofv@wanadoo.es>
To: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] master 6cd5678: Clarify compiler-pacifier in frame.c
Date: Mon, 26 Aug 2019 21:15:17 +0200	[thread overview]
Message-ID: <87blwb4u2i.fsf@telefonica.net> (raw)
In-Reply-To: 83a7bvg4a2.fsf@gnu.org

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Óscar Fuentes <ofv@wanadoo.es>
>> Date: Mon, 26 Aug 2019 20:20:23 +0200
>> 
>> Valgrind knows nothing about UNINIT as it works with machine code, not
>> with source code. But AFAIK that macro is conditionally defined to "=0"
>> (for silencing bogus gcc warnings) or to nothing (for leaving the
>> variable uninitalized at the declaration point). The later allows
>> Valgrind to do a proper check.
>> 
>> Simply changing
>> 
>> int x;
>> 
>> to
>> 
>> int x = 0;
>> 
>> just for silencing the gcc warning can hide a bug that Valgrind would
>> detect otherwise.
>
> This is backwards: it would mean we should use UNINIT all over the
> place just to be sure we will be able to spot some imaginary bugs by
> flipping a compiler switch.

UNINIT should only be used to avoid bogus warnings, *actual* bogus
warnings, not potential ones.

> The initialization in this case was added because GCC flagged a
> potential use-before-define.  After that, there's no more bug for
> Valgrind to find, so I see no reason to leave UNINIT around.

Let's suppose that the warning was correct and the hacker was wrong when
judged it bogus. Adding the initialization silences the warning *and*
Valgrind, while the UNINIT trick would cause Valgrind to complain (when
Valgrind examines an Emacs executable built with the options that cause
UNINIT to be empty.)

Or suppose that the warning is indeed bogus now. By adding the
initialization you are precluding Valgrind to find a bug if some future
code change introduces it.

> I could understand using UNINIT (or something similar) when the
> developer has no idea what not initializing could cause.  But this is
> not that case.

One of the most important things of the mental model of a C/C++ hacker
is the declare->initialize->use->dispose sequence of any storage. If the
developer is unsure about where to initialize a variable, he must stop
and think. The declaration should do the initialization only when it is
the right thing, that means that the variable must have that value from
that point. A variable shouldn't be assigned before the point where the
information it is supposed to store is available.

(Sorry if that sounds as if I were giving a lesson, that's not my
intention at all. I'm sure that you know what I explained by heart, but
it seems to me that you are not fully aware of the fact that some tools
do provide reliable detection of certain conditions that the compilers
tried half-assedly to detect for ages only to annoy experienced hackers
and force them to use workarounds that now are counterproductive.)




  parent reply	other threads:[~2019-08-26 19:15 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-24  6:14 [Emacs-diffs] master 6cd5678: Clarify compiler-pacifier in frame.c Eli Zaretskii
2019-08-25  0:52 ` Paul Eggert
2019-08-25  7:13   ` Eli Zaretskii
2019-08-26  6:34     ` Paul Eggert
2019-08-26  7:54       ` Eli Zaretskii
2019-08-26  8:15         ` Paul Eggert
2019-08-26  9:47           ` Eli Zaretskii
2019-08-26 15:21             ` Óscar Fuentes
2019-08-26 16:13               ` Eli Zaretskii
2019-08-26 18:20                 ` Óscar Fuentes
2019-08-26 18:39                   ` Eli Zaretskii
2019-08-26 19:09                     ` Paul Eggert
2019-08-26 19:15                     ` Óscar Fuentes [this message]
2019-08-26 19:33                       ` Eli Zaretskii
2019-08-26 19:49                         ` Óscar Fuentes
2019-08-26 22:33                         ` Paul Eggert
2019-08-27  6:12                           ` Eli Zaretskii
2019-08-27  7:28                             ` Paul Eggert
2019-08-27  8:02                               ` Eli Zaretskii
2019-08-27  9:28                                 ` Paul Eggert
2019-08-27 10:15                                   ` Eli Zaretskii
2019-08-27 12:05                                     ` Paul Eggert
2019-08-27 12:43                                       ` Eli Zaretskii
2019-08-26 18:50             ` Paul Eggert
2019-08-26 18:56               ` Eli Zaretskii
2019-08-26 23:17             ` Richard Stallman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blwb4u2i.fsf@telefonica.net \
    --to=ofv@wanadoo.es \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).