Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 May 2012 20:51:44 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Garrett Cooper <yanegomi@gmail.com>
Cc:        svn-src-head@freebsd.org, Kevin Lo <kevlo@freebsd.org>, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r235767 - head/lib/libc/net
Message-ID:  <20120522183847.M1344@besplex.bde.org>
In-Reply-To: <CAGH67wS90GGYHL%2Bjr1U3rO5rs%2BAark0jRHTQAqEMVTFXugiDAg@mail.gmail.com>
References:  <201205220128.q4M1SXPv081576@svn.freebsd.org> <CAGH67wS90GGYHL%2Bjr1U3rO5rs%2BAark0jRHTQAqEMVTFXugiDAg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--0-2035810234-1337683904=:1344
Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Mon, 21 May 2012, Garrett Cooper wrote:

> On Mon, May 21, 2012 at 6:28 PM, Kevin Lo <kevlo@freebsd.org> wrote:
>> Author: kevlo
>> Date: Tue May 22 01:28:32 2012
>> New Revision: 235767
>> URL: http://svn.freebsd.org/changeset/base/235767
>>
>> Log:
>> =A0Add missing header needed by free()
>>
>> =A0Reported by: =A0tinderbox

Please don't use binary characters in mail.

> Thank you for saving my mailbox :)!!!
>
> FWIW, it's weird because my STABLE-9 box didn't reproduce this.

This seems to be a bug in both nsparser.y and the new yacc:
- new yacc: it is incompatible.

   Old yacc includes stdlib.h and string.h as the first thing in the
   generated file, so the C code copied from the .y part sees them too.

   New yacc puts more its code including its all includes (which still
   involve stdlib.h) at the end of the generated file.

   Including stdlib.h nearly first is from Lite2 (skeleton.c 1.7 in 1997).
   Much later, FreeBSD moved it to the very first thing in the generated
   file, so as to use namespace pollution (__unused) from it early.
   __unused was ifdefed and was defined by the yacc skeleton if <stdlib.h>
   didn't define it.  It took much churn to produce this, but it seems to
   have been garbage, since old yacc didn't actually generate any use of
   __unused.  The definition was apparently compatibilty cruft to hide
   unportabilities in .y files that use __unused (I guess these failed for
   bootstrapping).  New yacc doesn't define __unused.

- nsparser.y: it uses free() but never declared it.  It depended on
   namespace pollution in the yacc code, and this pollution being in a
   particular order.

The yyparse() incompatibilities seem to be bugs on both sides too:
- Old yacc didn't declare yyparse() automatically, so .y files had to do
   it, but probably shouldn't have.

- New yacc still doesn't declare yylex() or yyerror() automatically, so
   applications still have to declare them.

- Some of these may be more the responsibility of the yacc library (to
   supply defaults) than others.  I couldn't see who is required to declare
   them in POSIX.1-2001.  However, a bad example in POSIX.1 has "extern int
   yyparse();" in main() just before calling yyparse().  Badness in this
   example include several style bugs and the declaration being incomplete
   (missing "void").
     Old yacc has about 20 lines of ifdefs half just to supply this "void"
     for STDC but not for K&R.  New yacc no longer supports K&R, at least
     here, but it still has 11 lines of ifdefs to get the declaration of
     yyparse() right for bison compatibility).
   I don't know if the badness includes having the home made declaration.

Nearby bugs: all CSRG and FreeBSD changes including copyrights and
history seem to have been lost.  Just after the old include of stdlib.h,
skeleton.c used to produce a string yyrcsid[] with __FBSDID() for
skeleton.c itself in it.  New yacc generates a string yysccsid with
a hard-coded berkeley 1993 id in it, and the name in this id doesn't
even match the file name (it is "yaccpar").  Another FreeBSD change was
to remove this bogus id from the generated file.

Some relevant FreeBSD changes:

% Index: skeleton.c
% =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
% RCS file: /home/ncvs/src/usr.bin/yacc/skeleton.c,v
% retrieving revision 1.1.1.2
% retrieving revision 1.37
% diff -u -2 -r1.1.1.2 -r1.37
% --- skeleton.c=096 Jan 1997 03:03:46 -0000=091.1.1.2
% +++ skeleton.c=0912 Feb 2003 18:03:55 -0000=091.37
% @@ -35,7 +35,12 @@
%   */
%=20
% +#if 0
%  #ifndef lint
%  static char sccsid[] =3D "@(#)skeleton.c=095.8 (Berkeley) 4/29/95";
% -#endif /* not lint */
% +#endif
% +#endif

FreeBSD normally comments out vendor ids like this.  5.8 is from Lite2;
Lite1 had 5.7.  New yacc doesn't have any of this.  It apparently started
from older yacc (before Lite1; indeed, it agrees with the FreeBSD-1 =3D=3D =
Net/2
version in not having any CSRG copyrights or this sccsid).

% +
% +#include <sys/cdefs.h>
% +__FBSDID("$FreeBSD: src/usr.bin/yacc/skeleton.c,v 1.37 2003/02/12 18:03:=
55 davidc Exp $");

FreeBSD normally adds this to source files.  Except in contrib.  yacc is
now mostly in contrib, although it hasn't changed all that much.

%=20
%  #include "defs.h"
% @@ -53,31 +58,42 @@
%  /*  are conditional.=09=09=09=09=09=09=09*/
%=20
% -char *banner[] =3D
% +const char *banner[] =3D

New yacc has this change too.

%  {
% +    "#include <stdlib.h>",

See above.

%      "#ifndef lint",
% -    "static char yysccsid[] =3D \"@(#)yaccpar=091.9 (Berkeley) 02/21/93\=
";",

Was in Net/2 (?), FreeBSD-1, Lite1 and Lite2.  Came back with new yacc.
It's interesting that "yaccpar 1.9 (Berkeley) 1.9 02/21/93" is only 3 month=
s
before the correct id "skeleton.c 5.7 (Berkeley) 5/24/93".

Hmm, the FreeBSD-1 history is even more instructive than I first
thought.  FreeBSD-1 started with the CSRG version of skeleton.c on
1993/06/12.  This had CSRG copyrights and ids, and yaccpar was at 1.8
01/20/90.  But this was upgraded to "the newest version on
vangogh.cs.berkeley.edu" 17 days later on 1993/06/29.  This version
has no CSRG copyrights or ids, except for the above rotted one for
yaccpar (the 1.9 02/21/93 one).  I think it was only missing CSRG
copyrights and ids because CSRG only added these to full releases.
This resulted in the version in FreeBSD-1 not having any copyright
notices inside most files, and none for the directory either.  The
new byacc seems to be the same as the older yacc here too.

% +    "#ifdef __unused",
% +    "__unused",
% +    "#endif",
% +    "static char const ",
% +    "yyrcsid[] =3D \"$FreeBSD: src/usr.bin/yacc/skeleton.c,v 1.37 2003/0=
2/12 18:03:55 davidc Exp $\";",
%      "#endif",
% -    "#include <stdlib.h>",

See above.

%      "#define YYBYACC 1",
%      "#define YYMAJOR 1",
%      "#define YYMINOR 9",
% -    "#define yyclearin (yychar=3D(-1))",
% +    "#define YYLEX yylex()",
% +    "#define YYEMPTY -1",
% +    "#define yyclearin (yychar=3D(YYEMPTY))",
%      "#define yyerrok (yyerrflag=3D0)",
% -    "#define YYRECOVERING (yyerrflag!=3D0)",
% +    "#define YYRECOVERING() (yyerrflag!=3D0)",
% +    "#if defined(__cplusplus) || __STDC__",
% +    "static int yygrowstack(void);",
% +    "#else",
% +    "static int yygrowstack();",
% +    "#endif",

%      0
%  };

Hrmph.  It was FreeBSD that added the STDC ifdefs, to avoid breaking
support for generating K&R code (older yacc generated fairly pure K&R
code without even "const").  For yygrowstack(), this was sloppy:
it only changed the declaration -- the function body still doesn't
say "void".

% @@ -151,13 +174,35 @@
%      "#define YYACCEPT goto yyaccept",
%      "#define YYERROR goto yyerrlab",
% +    "",
% +    "#ifndef YYPARSE_PARAM",
% +    "#if defined(__cplusplus) || __STDC__",
% +    "#define YYPARSE_PARAM_ARG void",
% +    "#define YYPARSE_PARAM_DECL",
% +    "#else=09/* ! ANSI-C/C++ */",
% +    "#define YYPARSE_PARAM_ARG",
% +    "#define YYPARSE_PARAM_DECL",
% +    "#endif=09/* ANSI-C/C++ */",
% +    "#else=09/* YYPARSE_PARAM */",
% +    "#ifndef YYPARSE_PARAM_TYPE",
% +    "#define YYPARSE_PARAM_TYPE void *",
% +    "#endif",
% +    "#if defined(__cplusplus) || __STDC__",
% +    "#define YYPARSE_PARAM_ARG YYPARSE_PARAM_TYPE YYPARSE_PARAM",
% +    "#define YYPARSE_PARAM_DECL",
% +    "#else=09/* ! ANSI-C/C++ */",
% +    "#define YYPARSE_PARAM_ARG YYPARSE_PARAM",
% +    "#define YYPARSE_PARAM_DECL YYPARSE_PARAM_TYPE YYPARSE_PARAM;",
% +    "#endif=09/* ANSI-C/C++ */",
% +    "#endif=09/* ! YYPARSE_PARAM */",
% +    "",
%      "int",
% -    "yyparse()",
% +    "yyparse (YYPARSE_PARAM_ARG)",
% +    "    YYPARSE_PARAM_DECL",
%      "{",
% -    "    register int yym, yyn, yystate;",
% +    "    int yym, yyn, yystate;",
%      "#if YYDEBUG",
% -    "    register char *yys;",
% -    "    extern char *getenv();",
% +    "    const char *yys;",
%      "",
% -    "    if (yys =3D getenv(\"YYDEBUG\"))",
% +    "    if ((yys =3D getenv(\"YYDEBUG\")))",
%      "    {",
%      "        yyn =3D *yys;",

The STDC support was especially messy for yyparse().

Bruce
--0-2035810234-1337683904=:1344--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120522183847.M1344>