Re: [PATCH MIPS] Work around Bash 4.2 bug

[Maciej W. Rozycki]

On Fri, 30 Sep 2016, Hans-Peter Nilsson wrote:

> > The patch below works around the Bash 4.2 bug described at
> > .
> 
> > * emulparams/elf32bmipn32-defs.sh: Shift quote of
> > "x$EMULATION_NAME" to the left to work around
> > .
> >
> 
> >
> > -case x"$EMULATION_NAME" in
> > +case "x$EMULATION_NAME" in
> >  xelf32*n32*) ELFSIZE=32 ;;
> 
> Random comment from the sideline: pretty please add a comment
> regarding the bug workaround *on top of the actual changed code*
> (remember: the "why" goes in the code, not in the changelog), so
> it has a better chance of not being inadvertently reverted but
> instead propagating elsewhere.

 CC-ing  as this might affect them too.

 Hmm, the shell construct is so common that I think rather than auditing 
all the scripts throughout our tree I'd rather made a `configure' check 
for the buggy shell feature and reject any shell affected at the top level 
and across subdirectories.  This way we won't have to keep an eye too for 
future script changes which might reintroduce the construct elsewhere.

 Nick, WDYT?

 Also for the purpose of such a check what does x"$EMULATION_NAME" expand 
to in the buggy case given a specific value of $EMULATION_NAME?

  Maciej

Re: [PATCH MIPS] Work around Bash 4.2 bug

[Ludovic Courtès]

"Maciej W. Rozycki"  skribis:

> On Fri, 30 Sep 2016, Hans-Peter Nilsson wrote:
>
>> > The patch below works around the Bash 4.2 bug described at
>> > .
>> 
>> >* emulparams/elf32bmipn32-defs.sh: Shift quote of
>> >"x$EMULATION_NAME" to the left to work around
>> >.
>> >
>> 
>> >
>> > -case x"$EMULATION_NAME" in
>> > +case "x$EMULATION_NAME" in
>> >  xelf32*n32*) ELFSIZE=32 ;;

[...]

>  Also for the purpose of such a check what does x"$EMULATION_NAME" expand 
> to in the buggy case given a specific value of $EMULATION_NAME?

Here’s a sample:

--8<---cut here---start->8---
$ echo $BASH_VERSION
4.2.0(1)-release
$ unset EMULATION_NAME
$ echo x$EMULATION_NAME
x
$ echo x"$EMULATION_NAME"
x
$ case x"$EMULATION_NAME" in x) echo good;; *) echo bad;; esac
bad
$ case "x$EMULATION_NAME" in x) echo good;; *) echo bad;; esac
good
$ echo x"$EMULATION_NAME"y
xy
$ echo "x$EMULATION_NAME"y
xy
--8<---cut here---end--->8---

IOW, the problem manifests in ‘case’ statements, but not in ‘echo’.

I agree that rejecting such buggy shells at configure time would be
safer.

There’s been 1 year between Bash 4.2 and Bash 4.3, and 3 weeks between
Bash 4.2 and the ‘bash42-007’ fix; not sure how widespread the buggy
shell is.

HTH,
Ludo’.

style convention: /*foo_p=*/ to annotate bool arguments

[Martin Sebor]

In a recent review Jason and I discussed the style convention
commonly followed in the C++ front end to annotate arguments
in calls to functions taking bool parameters with a comment
along the lines of

  foo (1, 2, /*bar_p=*/true);

I pointed out to Jason that in another code review, Jeff asked
to remove the same comment from the middle-end [1].  In the
interest of consistency Jason and I thought I should bring this
up for discussion so we can all be clear on whether or not this
is something worth standardizing and documenting.

As a separate question, in the same discussion I mention to Jason
a convention that I myself have found useful where the value of
a default argument is indicated in a comment in a function
definition that is declared elsewhere to take one, e.g., like so:

  // In some header:
  void foo (int, int, bool = -1);

  // In some .c file:
  void foo (int x, int y, bool bar_p /* = false */)
  {
...
  }

Jason thought this would be a good convention.  Is there any
interest in/support for adopting it?

Thanks
Martin

[1] https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00354.html
[2] https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01469.html