Re: [MTASCsft PATCH 38/38] MT-, AS- and AC-Safety docs: sanity check

["Carlos O'Donell" <carlos at redhat dot com>]

On 01/27/2014 11:35 PM, Alexandre Oliva wrote:
> This is the last patch in the MTASCsft patchset.  It's the first time I
> post this one.  I'm not sure it ought to go in, but it's helped me catch
> a number of mistakes.
> 
> 
> Check safety remarks in the manual.

This looks great and is very cool!

Some nits though.
 
> for  ChangeLog
> 
> 	* manual/check-safety.sh: New.
> 	* manual/Makefile ($(objpfx)stamp-summary): Run it.
> ---
>  manual/Makefile        |    3 ++-
>  manual/check-safety.sh |   49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 1 deletion(-)
>  create mode 100755 manual/check-safety.sh
> 
> diff --git a/manual/Makefile b/manual/Makefile
> index 3037303..4cb6807 100644
> --- a/manual/Makefile
> +++ b/manual/Makefile
> @@ -88,6 +88,7 @@ $(objpfx)libc/index.html: $(addprefix $(objpfx),$(libc-texi-generated))
>  $(objpfx)summary.texi: $(objpfx)stamp-summary ;
>  $(objpfx)stamp-summary: summary.awk $(filter-out $(objpfx)summary.texi, \
>  					$(texis-path))
> +	$(SHELL) ./check-safety.sh $(filter-out $(objpfx)%, $(texis-path))

OK.

>  	$(AWK) -f $^ | sort -t'' -df -k 1,1 | tr '\014' '\012' \
>  		> $(objpfx)summary-tmp
>  	$(move-if-change) $(objpfx)summary-tmp $(objpfx)summary.texi
> @@ -157,7 +158,7 @@ $(objpfx)%.pdf: %.texinfo
>  
>  # Distribution.
>  minimal-dist = summary.awk texis.awk tsort.awk libc-texinfo.sh libc.texinfo \
> -	       libm-err.texi stamp-libm-err				    \
> +	       libm-err.texi stamp-libm-err check-safety.sh		    \

OK.

>  	       $(filter-out summary.texi, $(nonexamples))		    \
>  	       $(patsubst %.c.texi,examples/%.c, $(examples))
>  
> diff --git a/manual/check-safety.sh b/manual/check-safety.sh
> new file mode 100755
> index 0000000..616a232
> --- /dev/null
> +++ b/manual/check-safety.sh
> @@ -0,0 +1,49 @@
> +#! /bin/sh
> +
> +# Copyright 2014 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <http://www.gnu.org/licenses/>.
> +
> +
> +# Check that the @safety notes are self-consistent.

Please expand the comment as I have no idea what we're actually
checking for here. It looks like you're encoding rules about
which markers can be used with which safety notes.
> +
> +success=:
> +
> +test $# != 0 || set *.texi
> +

Each of these needs a comment saying what it's doing.

> +grep -n '^@safety' "$@" | grep -v ':@safety{\(@prelim{}\)\?@mt\(un\)\?safe{.*}@as\(un\)\?safe{.*}@ac\(un\)\?safe{.*}}' && success=false

There has to be a way to make these lines < 80 chars. My eyes are bleeding.

> +
> +grep -n '^@safety' "$@" | grep -v ':@safety{\(@prelim{}\)\?@mt\(un\)\?safe{\(@mt\(asc\?\|ac\)\?[su][^ ]*}\)\?\( @mt\(asc\?\|ac\)\?[su][^ ]*}\)*}@as\(un\)\?safe{\(@asc\?[su][^ ]*}\)\?\( @asc\?[su][^ ]*}\)*}@ac\(un\)\?safe{\(@ac[su][^ ]*}\)\?\( @ac[su][^ ]*}\)*}}' && success=false
> +
> +grep -n '^@safety.*@mtsafe' "$@" | grep '@mt\(asc\?\|ac\)?u' "$@" && success=false
> +
> +grep -n '^@safety.*@mtunsafe' "$@" | grep -v '@mtunsafe{.*@mt\(asc\?\|ac\)\?u' && success=false
> +
> +grep -n '^@safety.*@assafe' "$@" | grep '@\(mt\)\?asc\?u' && success=false
> +
> +grep -n '^@safety.*@asunsafe' "$@" | grep -v '@mtasc\?u.*@asunsafe\|@asunsafe{.*@asc\?u' && success=false
> +
> +grep -n '^@safety.*@acsafe' "$@" | grep '@\(mt\)\?as\?cu' && success=false
> +
> +grep -n '^@safety.*@acunsafe' "$@" | grep -v '@\(mtas\?\|as\)cu.*@acunsafe\|@acunsafe{.*@acu' && success=false
> +
> +grep -n '^@safety' "$@" | grep '[^:]\(@\(mt\|a[sc]\)[^ {]*{[^ ]*}\).*[^:]\1' && success=false
> +
> +grep -n '^@c \+[^@ ]\+\( dup\)\?\( @\(mt\|a[sc]\)[^ ]*\)*\( \[.*\]\)\?$' "$@" | grep -v ':@c *[^@{}]*\( @mt[^ {}]*\)*\( @as[^ {}]*\)*\( @ac[^ {}]*\)*\( \[.*\]\)\?$' && success=false
> +
> +grep -n '^@c \+[^@ ]\+\( dup\)\?\( @\(mt\|a[sc]\)[^ ]*\)*\( \[.*\]\)\?$' "$@" | grep '[^:]\(@\(mt\|a[sc]\)[^ ]*\) \(.*[^:]\)\?\1\($\| \)' && success=false
> +
> +$success
> 

OK to checkin if you:

* Expand top comment.

* Add one line comment for each regexp.

* Try hard to get them under 80 chars, either by rewriting
  or using variables to assemble the regexps.

Cheers,
Carlos.