[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c
From: |
Jim Meyering |
Subject: |
Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c |
Date: |
Mon, 23 Jan 2012 12:35:39 +0100 |
Akim Demaille wrote:
> Le 23 janv. 2012 à 11:57, Jim Meyering a écrit :
>
>> Hi Akim,
>>
>> How about this?
>
> Actually, for mhistorical reason, yyerror is really expected
> to return an int (by default, which is the case here). I guess
> it dates back to the good ol' time o' C when not declaring
> was fine, and meant int.
>
> http://pubs.opengroup.org/onlinepubs/7908799/xcu/yacc.html
>
>> The following functions appear only in the yacc library accessible
>> through the -l y operand to cc or c89; they can therefore be
>> redefined by a portable application:
>> int main(void)
>> This function will call yyparse() and exit with an unspecified
>> value. Other actions within this function are unspecified.
>> int yyerror(const char *s
>> This function will write the NUL-terminated argument to standard
>> error, followed by a newline character.
>
>
>
>
>> I didn't find anyone using the return value, so rather than
>> trying to preserve semantics for nonexistent callers, I opted
>> to make this yyerror function return void as documented.
>
> What do you mean by "as documented"?
The one the manual suggest we provide in examples, e.g.,
Let's consider an example, vastly simplified from a C++ grammar.
%{
#include <stdio.h>
#define YYSTYPE char const *
int yylex (void);
void yyerror (char const *);
%}
>> Oh, wait!
>> I do see one non-void use. It's in data/glr.c:
>>
>> return yyerror (]b4_yyerror_args[YY_("syntax error: cannot back up")), \
>>
>> From the context, I'm not sure if that code is ever used:
>>
>> ------------------------------------------
>> # undef YYBACKUP
>> # define YYBACKUP(Token, Value)
>> \
>> return yyerror (]b4_yyerror_args[YY_("syntax error: cannot back up")),
>> \
>> yyerrok, yyerr
>
> I don't see what you mean here: the return value is ignored by ','.
Good! I was not reading carefully.
>> Anyhow, with that I'm not so sure it's ok to s/int/void/.
>
> I don't think it is. But there is no semantics attached to that
> int. 0 would do fine.
>
>> * lib/yyerror.c (yyerror): Use fputs and fputc rather than fprintf
>> with a mere "%s\n" format. Also, change the return type to void.
>> This avoids a problem reported by Thiru Ramakrishnan in
>> http://lists.gnu.org/archive/html/help-bison/2011-11/msg00000.html
>> ---
>> lib/yyerror.c | 7 ++++---
>> 1 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/yyerror.c b/lib/yyerror.c
>> index 5eb339f..5ac0438 100644
>> --- a/lib/yyerror.c
>> +++ b/lib/yyerror.c
>> @@ -20,10 +20,11 @@
>> #include <config.h>
>> #include <stdio.h>
>>
>> -int yyerror (char const *);
>> +void yyerror (char const *);
>>
>> -int
>> +void
>> yyerror (char const *message)
>> {
>> - return fprintf (stderr, "%s\n", message);
>> + fputs (message, stderr);
>> + fputc ('\n', stderr);
>> }
>
> So you are engaging yourself gnulib will never #define these guys? :)
> Actually, is main.c's setlocale's also protected from such dependencies?
I now think it's better not to include <config.h> there, after all.
fputs and fputc are far less likely to require replacement.
Re setlocale, let's defer crossing that bridge until we come to it.
I don't expect gnulib will replace it any time soon.
>> #include <config.h>
>>
>> #if HAVE_LOCALE_H
>> # include <locale.h>
>> #endif
>> #if ! HAVE_SETLOCALE
>> # define setlocale(Category, Locale)
>> #endif
>>
>> int yyparse (void);
>>
>> int
>> main (void)
>> {
>> setlocale (LC_ALL, "");
>> return yyparse ();
>> }
Thanks for the reality check.
How about this revised patch?
>From 507aafd6b2388780aad6dfd349c65075c2b378e6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 23 Jan 2012 11:47:46 +0100
Subject: [PATCH] build: avoid possibly-replaced fprintf in liby-source,
yyerror.c
* lib/yyerror.c (yyerror): Use fputs and fputc rather than fprintf
with a mere "%s\n" format. Always return 0 now, on the assumption
that the return value was never used anyway.
Don't include <config.h> after all. This avoids a problem
reported by Thiru Ramakrishnan in
http://lists.gnu.org/archive/html/help-bison/2011-11/msg00000.html
* cfg.mk: Exempt lib/yyerror.c from the sc_require_config_h_first test.
---
cfg.mk | 3 ++-
lib/yyerror.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 2d4f1ba..6b3deb9 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -55,4 +55,5 @@ update-copyright-env = \
UPDATE_COPYRIGHT_FORCE=1 UPDATE_COPYRIGHT_USE_INTERVALS=1
exclude_file_name_regexp--sc_space_tab = ^tests/(input|c\+\+)\.at$$
-exclude_file_name_regexp--sc_require_config_h_first = ^data/(glr|yacc)\.c$$
+exclude_file_name_regexp--sc_require_config_h_first = \
+ ^(lib/yyerror|data/(glr|yacc))\.c$$
diff --git a/lib/yyerror.c b/lib/yyerror.c
index 5eb339f..c9f492f 100644
--- a/lib/yyerror.c
+++ b/lib/yyerror.c
@@ -17,7 +17,6 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#include <config.h>
#include <stdio.h>
int yyerror (char const *);
@@ -25,5 +24,7 @@ int yyerror (char const *);
int
yyerror (char const *message)
{
- return fprintf (stderr, "%s\n", message);
+ fputs (message, stderr);
+ fputc ('\n', stderr);
+ return 0;
}
--
1.7.9.rc2.2.g183d6
- syntax-check, update bootstrap, update gnulib, Jim Meyering, 2012/01/18
- [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Jim Meyering, 2012/01/18
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/18
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Jim Meyering, 2012/01/18
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/23
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Jim Meyering, 2012/01/23
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/23
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c,
Jim Meyering <=
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/23
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Jim Meyering, 2012/01/23
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/25
- Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Paul Eggert, 2012/01/23
Re: [PATCH 06/11] build: include <config.h> from lib/yyerror.c, Akim Demaille, 2012/01/23
[PATCH 03/11] maint: remove final trailing space, Jim Meyering, 2012/01/18
[PATCH 05/11] maint: list djgpp/subpipe.c in po/POTFILES.in, Jim Meyering, 2012/01/18