Re: several buglets in CVS m4

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Ralf,

According to Ralf Wildenhues on 6/16/2006 1:33 AM:
> [ Please keep me in Cc:, I am not subscribed. ]
> 
> Hello Eric, Gary,  ;-)
> 
> Here's a list of suspicious things and easy patches.  I've not yet had
> time to look into the more difficult ones yet, sorry about that.

Thanks for the good list.

> 
> 1) The modules/gnu.la library has the global unprefixed symbol `format'.
> This looks like a bug to me.
> 
> 2) modules/m4.la has the global symbol m4_sysval, so has the m4_ prefix
> but not the m4_LTX_m4_ prefix.

But you didn't provide a patch for these issues?

> 
> 3) Automake creates build tree directory rules for the targets it writes
> the rules for (the .dirstamp stuff), but not for manually written rules;
> this needs to be done for doc/m4.1 (for a VPATH build).  Patch below,
> written not to depend on the Automake internal detail .dirstamp.

Thanks for catching that; I don't have easy access to a non-GNU make for
VPATH testing.

> 
> 4) GCC 4.0 has dropped the cast-as-lvalue extension, needing a change in 
> src/stackovf.c.  Patch below.

But why do we need the cast at all?  Don't all pointers automatically
promote to void*?

> 
> 5) GCC 4.1 uncovered a potential bug in reload_frozen_state: with
> suitable settings M4ERROR may not exit the program but suppress the
> warning (is that correct, by the way, or is that the real bug?), leaving
> the following code to potentially read uninitialized memory.  Suggested
> patch below, but please make sure this is what is intended.

Indeed, there's a bigger set of issues here, and it affects 1.4.x as well
(although I'm less inclined to change status quo on the 1.4.x series).
GNU Coding Standards state that error messages should be

program:file:line: message

But M4ERROR is doing

file:line: program: Message

GCS recommends that 'program:' be elided from interactive programs (when
the message appears on the same terminal as the person typing in the bad
input), but 'm4 -e' still prints its program name.

POSIX requires that if an error message is printed to stderr, then the
program must exit (eventually) with non-zero status.  In general, it is a
nice extension that m4 can provide warnings; in part because the -E option
can turn them into errors or -Q can suppress them (although to be strictly
POSIX compliant, POSIXLY_CORRECT or -G should imply -Q).  But right now,
m4 does a very bad job about remembering when an error (not warning) was
printed, and usually has a successful exit status in spite of the error
messages; partly because m4 tries so hard in many locations to continue in
spite of an error.

It will take a full audit of M4ERROR to clean this up properly, and that
is on my list of things to eventually get to.

Now, as to your report, M4ERROR is guaranteed to call exit() if the first
argument is non-zero (and EXIT_FAILURE qualifies), so your concern about
number[2] being used uninitialized is unfounded.  However, I agree that
silencing the warning from the compiler is still useful.

> 
> Cheers,
> Ralf
> 
>       * Makefile.am (doc/m4.1): Create the doc directory if needed.
> 
>       * src/stackovf.c (stackovf_exit): Move cast to `void *'...
>       * m4/m4private.h (DELETE): ...here, to avoid unportable
>       cast-as-lvalue.
> 
>       * src/freeze.c (reload_frozen_state): Initialize `number' in
>       case M4ERROR does not exit the program.

I'll post the patch that I eventually apply.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEkrJa84KuGfSFAYARAr1pAJ0YscpvdY2VtQ6ksOsZrEB/38b8uQCgiCLz
IESn3b2au940sjsmoyijm5M=
=CQQ7
-----END PGP SIGNATURE-----