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
[-- Attachment #1 --] On Thu, Nov 16, 2023 at 9:07 AM 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=accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d > > > > 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 the > > 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 of > > 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= > > - temp=$(mktemp -d genoffset.XXXXXXXXXX) > > - trap "rm -rf ${temp}" EXIT > > - # Note: we need to print symbol values in decimal so the numeric > sort works > > - ${NM:='nm'} ${NMFLAGS} -t d "$1" | grep __assym_offset__ | sed -e > 's/__/ /g' | sort -k 4 -k 1 -n | > > + asserts= > > while read off x1 x2 struct field type x3; do > > off=$(echo "$off" | sed -E 's/^0+//') > > if [ "$last" != "$struct" ]; then > > @@ -60,12 +57,14 @@ work() > > printf "%b" "\t${type}\t${field};\n" > > lastoff="$off" > > lasttype="$type" > > - echo "_SA(${struct}, ${field}, ${off});" >> "$temp/asserts" > > - done > > + asserts="${asserts}_SA(${struct}, ${field}, ${off});\n" > > + done <<EOT > > +$(${NM:='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) == o, \\" > > printf '\t"struct "#s"_lite field "#f" not at offset "#o)\n' > > - cat "$temp/asserts" > > + echo -e "${asserts}\c" > > This isn’t POSIX, and isn’t supported by macOS’s 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" > > [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 16, 2023 at 9:07 AM Jessica Clarke <<a href="mailto:jrtc27@freebsd.org">jrtc27@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 16 Nov 2023, at 15:07, Jonathan T. Looney <jtl@FreeBSD.org> wrote:<br> > <br> > The branch main has been updated by jtl:<br> > <br> > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d</a><br>; > <br> > commit accfb4cc9346b23f6d6383dfc98d2c97ae18ce0d<br> > Author: Jonathan T. Looney <jtl@FreeBSD.org><br> > AuthorDate: 2023-11-16 15:02:32 +0000<br> > Commit: Jonathan T. Looney <jtl@FreeBSD.org><br> > CommitDate: 2023-11-16 15:02:32 +0000<br> > <br> > genoffset.sh: stop using a temporary file<br> > <br> > Instead, use a here document for the input. This allows us to run the<br> > while loop in the main script so we can build the list of asserts in<br> > a shell variable. We then print out the list of asserts at the end of<br> > the loop.<br> > <br> > Reviewed by: imp<br> > Sponsored by: Netflix<br> > Differential Revision: <a href="https://reviews.freebsd.org/D42407" rel="noreferrer" target="_blank">https://reviews.freebsd.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> > - local last off x1 x2 x3 struct field type lastoff lasttype<br> > + local last off x1 x2 x3 struct field type lastoff lasttype asserts<br> > <br> > echo "#ifndef _OFFSET_INC_"<br> > echo "#define _OFFSET_INC_"<br> > echo "#if !defined(GENOFFSET) && (!defined(KLD_MODULE) || defined(KLD_TIED))"<br> > last=<br> > - temp=$(mktemp -d genoffset.XXXXXXXXXX)<br> > - trap "rm -rf ${temp}" EXIT<br> > - # Note: we need to print symbol values in decimal so the numeric sort works<br> > - ${NM:='nm'} ${NMFLAGS} -t d "$1" | grep __assym_offset__ | sed -e 's/__/ /g' | sort -k 4 -k 1 -n |<br> > + asserts=<br> > while read off x1 x2 struct field type x3; do<br> > off=$(echo "$off" | sed -E 's/^0+//')<br> > if [ "$last" != "$struct" ]; then<br> > @@ -60,12 +57,14 @@ work()<br> > printf "%b" "\t${type}\t${field};\n"<br> > lastoff="$off"<br> > lasttype="$type"<br> > - echo "_SA(${struct}, ${field}, ${off});" >> "$temp/asserts"<br> > - done<br> > + asserts="${asserts}_SA(${struct}, ${field}, ${off});\n"<br> > + done <<EOT<br> > +$(${NM:='nm'} ${NMFLAGS} -t d "$1" | grep __assym_offset__ | sed -e 's/__/ /g' | sort -k 4 -k 1 -n)<br> > +EOT<br> > echo "};"<br> > echo "#define _SA(s,f,o) _Static_assert(__builtin_offsetof(struct s ## _lite, f) == o, \\"<br> > printf '\t"struct "#s"_lite field "#f" not at offset "#o)\n'<br> > - cat "$temp/asserts"<br> > + echo -e "${asserts}\c"<br> <br> This isn’t POSIX, and isn’t supported by macOS’s 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 the 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). 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.</div><div><br></div><div>That's what I'd do :)</div><div><br></div><div>Warner</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Jess<br> <br> > echo "#undef _SA"<br> > echo "#endif"<br> > echo "#endif"<br> <br> </blockquote></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfr9G_jttwu8G85%2Bvcodf6SNnmt7WPhp8-M10vDrEroP2A>