[illumos-Developer] Webrev for bug 844: m4 should say something useful...

[Gary Mills]

On Sun, Apr 03, 2011 at 02:58:24PM -0700, Garrett D'Amore wrote:
> On Sun, 2011-04-03 at 10:40 -0500, Gary Mills wrote:
> > 
> > Okay, there's a new webrev at:
> > 
> >     http://cr.illumos.org/view/k9zwpc1c/illumos844-1/
> > 
> > that uses the string you recommended.  Here's how it looks now:
> > 
> >     $ usr/src/cmd/sgs/m4/i386/m4 sample.m4
> >     
> >     usr/src/cmd/sgs/m4/i386/m4:sample.m4:2 cannot open file: No such file or directory
> >     
> >     include(foobar.m4)
> >     $ env LC_MESSAGES=fr_CA.UTF-8 usr/src/cmd/sgs/m4/i386/m4 samp>
> >     
> >     usr/src/cmd/sgs/m4/i386/m4:sample.m4:2 impossible d'ouvrir le fichier : Ce fichier ou ce répertoire n'existe pas
> > 
> >     include(foobar.m4)
> > 
> > It is translated now.  Because of the `\n' in the new string, there is
> > an extra newline in the error message.  I'm not concerned about that.
> 
> Huh?  There should only be one new line.

It's because of the layering.  The code itself calls error(),
error2(), or now errorf() to report errors.  All of them take a string
that has no trailing newline as the first argument.  error2() inserts
an integer whereas errorf() inserts a string before calling error().
Finally, this function prints to stderr, appending a newline to the
complete string.

> > Is this change ready to go now?
> 
> I'd like to understand what you meant by an "extra" newline.  The code
> looks good to me, though honestly I'd have simply opted for perror(3c)
> instead of inventing a new function. E.g.
> 
> 	if (fopen(...) == NULL) {
> 		perror(gettext("cannot open file"));
> 		...
> 	}
> 
> perror looks up the errno for you, and also dumps the result to stderr.

That would violate the layering.  The real problem is that

    "cannot open file: %s\n"

is already in the message catalog, but the same string without the
trailing newline seems not to be there.  I don't know what the
convention is, but all the error strings in m4 have no trailing
newline.

> If you don't to make the switch to use perror, that's OK, its just not
> quite as simple/clean as if you use perror().

Okay, what's my next step to get this fix integrated?

-- 
-Gary Mills-        -Unix Group-        -Computer and Network Services-