Date: Thu, 16 Nov 2023 09:23:43 -0700 From: Warner Losh <imp@bsdimp.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: "Jonathan T. Looney" <jtl@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: accfb4cc9346 - main - genoffset.sh: stop using a temporary file Message-ID: <CANCZdfr9G_jttwu8G85%2Bvcodf6SNnmt7WPhp8-M10vDrEroP2A@mail.gmail.com> In-Reply-To: <39921E4A-4C15-4D8F-BA9F-D0C53AD62CD1@freebsd.org> References: <202311161507.3AGF7kgx070201@gitrepo.freebsd.org> <39921E4A-4C15-4D8F-BA9F-D0C53AD62CD1@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--00000000000030fa20060a477232 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Nov 16, 2023 at 9:07=E2=80=AFAM Jessica Clarke <jrtc27@freebsd.org>= wrote: > On 16 Nov 2023, at 15:07, Jonathan T. Looney <jtl@FreeBSD.org> wrote: > > > > The branch main has been updated by jtl: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3Daccfb4cc9346b23f6d6383dfc98d2c9= 7ae18ce0d > > > > commit accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d > > Author: Jonathan T. Looney <jtl@FreeBSD.org> > > AuthorDate: 2023-11-16 15:02:32 +0000 > > Commit: Jonathan T. Looney <jtl@FreeBSD.org> > > CommitDate: 2023-11-16 15:02:32 +0000 > > > > genoffset.sh: stop using a temporary file > > > > Instead, use a here document for the input. This allows us to run th= e > > while loop in the main script so we can build the list of asserts in > > a shell variable. We then print out the list of asserts at the end o= f > > the loop. > > > > Reviewed by: imp > > Sponsored by: Netflix > > Differential Revision: https://reviews.freebsd.org/D42407 > > --- > > sys/kern/genoffset.sh | 15 +++++++-------- > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/sys/kern/genoffset.sh b/sys/kern/genoffset.sh > > index fda27998ca79..c974a7d52e8c 100644 > > --- a/sys/kern/genoffset.sh > > +++ b/sys/kern/genoffset.sh > > @@ -35,16 +35,13 @@ usage() > > > > work() > > ( > > - local last off x1 x2 x3 struct field type lastoff lasttype > > + local last off x1 x2 x3 struct field type lastoff lasttype asserts > > > > echo "#ifndef _OFFSET_INC_" > > echo "#define _OFFSET_INC_" > > echo "#if !defined(GENOFFSET) && (!defined(KLD_MODULE) || > defined(KLD_TIED))" > > last=3D > > - temp=3D$(mktemp -d genoffset.XXXXXXXXXX) > > - trap "rm -rf ${temp}" EXIT > > - # Note: we need to print symbol values in decimal so the numeric > sort works > > - ${NM:=3D'nm'} ${NMFLAGS} -t d "$1" | grep __assym_offset__ | sed -= e > 's/__/ /g' | sort -k 4 -k 1 -n | > > + asserts=3D > > while read off x1 x2 struct field type x3; do > > off=3D$(echo "$off" | sed -E 's/^0+//') > > if [ "$last" !=3D "$struct" ]; then > > @@ -60,12 +57,14 @@ work() > > printf "%b" "\t${type}\t${field};\n" > > lastoff=3D"$off" > > lasttype=3D"$type" > > - echo "_SA(${struct}, ${field}, ${off});" >> "$temp/asserts" > > - done > > + asserts=3D"${asserts}_SA(${struct}, ${field}, ${off});\n" > > + done <<EOT > > +$(${NM:=3D'nm'} ${NMFLAGS} -t d "$1" | grep __assym_offset__ | sed -e > 's/__/ /g' | sort -k 4 -k 1 -n) > > +EOT > > echo "};" > > echo "#define _SA(s,f,o) _Static_assert(__builtin_offsetof(struct s > ## _lite, f) =3D=3D o, \\" > > printf '\t"struct "#s"_lite field "#f" not at offset "#o)\n' > > - cat "$temp/asserts" > > + echo -e "${asserts}\c" > > This isn=E2=80=99t POSIX, and isn=E2=80=99t supported by macOS=E2=80=99s = sh, so breaks the > build there. Please fix or revert promptly. > echo "${asserts}" is semantically the same for C. A stray newline doesn't matter in this context. It's not worth the effort to remove it. Sadly, echo -n doesn't work (it's not posix, and posix defines it specifically as implementation defined). `echo "fred\c"' works on macos, but not FreeBSD's shell echo built-in (but somehow does for /bin/echo). macos doesn't implement -e at all (it's also not posix). FreeBSD's shell echo build-in is not posix compliant. So, to be portable, just echo it, and cope with the extra newline. That's what I'd do :) Warner > Jess > > > echo "#undef _SA" > > echo "#endif" > > echo "#endif" > > --00000000000030fa20060a477232 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Thu, Nov 16, 2023 at 9:07=E2=80=AF= AM Jessica Clarke <<a href=3D"mailto:jrtc27@freebsd.org">jrtc27@freebsd.= org</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"marg= in:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e= x">On 16 Nov 2023, at 15:07, Jonathan T. Looney <jtl@FreeBSD.org> wro= te:<br> > <br> > The branch main has been updated by jtl:<br> > <br> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3Daccfb4cc9346= b23f6d6383dfc98d2c97ae18ce0d" rel=3D"noreferrer" target=3D"_blank">https://= cgit.FreeBSD.org/src/commit/?id=3Daccfb4cc9346b23f6d6383dfc98d2c97ae18ce0d<= /a><br> > <br> > commit accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d<br> > Author:=C2=A0 =C2=A0 =C2=A0Jonathan T. Looney <jtl@FreeBSD.org><= br> > AuthorDate: 2023-11-16 15:02:32 +0000<br> > Commit:=C2=A0 =C2=A0 =C2=A0Jonathan T. Looney <jtl@FreeBSD.org><= br> > CommitDate: 2023-11-16 15:02:32 +0000<br> > <br> >=C2=A0 =C2=A0 genoffset.sh: stop using a temporary file<br> > <br> >=C2=A0 =C2=A0 Instead, use a here document for the input. This allows u= s to run the<br> >=C2=A0 =C2=A0 while loop in the main script so we can build the list of= asserts in<br> >=C2=A0 =C2=A0 a shell variable. We then print out the list of asserts a= t the end of<br> >=C2=A0 =C2=A0 the loop.<br> > <br> >=C2=A0 =C2=A0 Reviewed by:=C2=A0 =C2=A0 imp<br> >=C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0Netflix<br> >=C2=A0 =C2=A0 Differential Revision:=C2=A0 <a href=3D"https://reviews.f= reebsd.org/D42407" rel=3D"noreferrer" target=3D"_blank">https://reviews.fre= ebsd.org/D42407</a><br> > ---<br> > sys/kern/genoffset.sh | 15 +++++++--------<br> > 1 file changed, 7 insertions(+), 8 deletions(-)<br> > <br> > diff --git a/sys/kern/genoffset.sh b/sys/kern/genoffset.sh<br> > index fda27998ca79..c974a7d52e8c 100644<br> > --- a/sys/kern/genoffset.sh<br> > +++ b/sys/kern/genoffset.sh<br> > @@ -35,16 +35,13 @@ usage()<br> > <br> > work()<br> > (<br> > -=C2=A0 =C2=A0 local last off x1 x2 x3 struct field type lastoff lastt= ype<br> > +=C2=A0 =C2=A0 local last off x1 x2 x3 struct field type lastoff lastt= ype asserts<br> > <br> >=C2=A0 =C2=A0 =C2=A0echo "#ifndef _OFFSET_INC_"<br> >=C2=A0 =C2=A0 =C2=A0echo "#define _OFFSET_INC_"<br> >=C2=A0 =C2=A0 =C2=A0echo "#if !defined(GENOFFSET) && (!def= ined(KLD_MODULE) || defined(KLD_TIED))"<br> >=C2=A0 =C2=A0 =C2=A0last=3D<br> > -=C2=A0 =C2=A0 temp=3D$(mktemp -d genoffset.XXXXXXXXXX)<br> > -=C2=A0 =C2=A0 trap "rm -rf ${temp}" EXIT<br> > -=C2=A0 =C2=A0 # Note: we need to print symbol values in decimal so th= e numeric sort works<br> > -=C2=A0 =C2=A0 ${NM:=3D'nm'} ${NMFLAGS} -t d "$1" | = grep __assym_offset__ | sed -e 's/__/ /g' | sort -k 4 -k 1 -n |<br> > +=C2=A0 =C2=A0 asserts=3D<br> >=C2=A0 =C2=A0 =C2=A0while read off x1 x2 struct field type x3; do<br> > off=3D$(echo "$off" | sed -E 's/^0+//')<br> > if [ "$last" !=3D "$struct" ]; then<br> > @@ -60,12 +57,14 @@ work()<br> > printf "%b" "\t${type}\t${field};\n"<br> > lastoff=3D"$off"<br> > lasttype=3D"$type"<br> > - echo "_SA(${struct}, ${field}, ${off});" >> "$t= emp/asserts"<br> > -=C2=A0 =C2=A0 done<br> > + asserts=3D"${asserts}_SA(${struct}, ${field}, ${off});\n"<= br> > +=C2=A0 =C2=A0 done <<EOT<br> > +$(${NM:=3D'nm'} ${NMFLAGS} -t d "$1" | grep __assym= _offset__ | sed -e 's/__/ /g' | sort -k 4 -k 1 -n)<br> > +EOT<br> >=C2=A0 =C2=A0 =C2=A0echo "};"<br> >=C2=A0 =C2=A0 =C2=A0echo "#define _SA(s,f,o) _Static_assert(__buil= tin_offsetof(struct s ## _lite, f) =3D=3D o, \\"<br> >=C2=A0 =C2=A0 =C2=A0printf '\t"struct "#s"_lite fiel= d "#f" not at offset "#o)\n'<br> > -=C2=A0 =C2=A0 cat "$temp/asserts"<br> > +=C2=A0 =C2=A0 echo -e "${asserts}\c"<br> <br> This isn=E2=80=99t POSIX, and isn=E2=80=99t supported by macOS=E2=80=99s sh= , so breaks the<br> build there. Please fix or revert promptly.<br></blockquote><div><br></div>= <div>echo "${asserts}"</div><div><br></div><div>is semantically t= he same for C. A stray newline doesn't matter in this context. It's= not worth the effort to remove it.</div><div><br></div><div>Sadly, echo -n= doesn't work (it's not posix, and posix defines it specifically as= implementation defined). `echo "fred\c"' works on macos, but= not FreeBSD's shell echo built-in (but somehow does for /bin/echo). ma= cos doesn't implement -e at all (it's also not posix). FreeBSD'= s shell echo build-in is not posix compliant. So, to be portable, just echo= it, and cope with the extra newline.</div><div><br></div><div>That's w= hat I'd do :)</div><div><br></div><div>Warner</div><div>=C2=A0</div><bl= ockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-lef= t:1px solid rgb(204,204,204);padding-left:1ex"> Jess<br> <br> >=C2=A0 =C2=A0 =C2=A0echo "#undef _SA"<br> >=C2=A0 =C2=A0 =C2=A0echo "#endif"<br> >=C2=A0 =C2=A0 =C2=A0echo "#endif"<br> <br> </blockquote></div></div> --00000000000030fa20060a477232--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfr9G_jttwu8G85%2Bvcodf6SNnmt7WPhp8-M10vDrEroP2A>