Bug 51488

On Mon, Apr 28, 2003 at 02:44:04PM +0400, Dmitry Sivachenko wrote:
> 
> >Number:         51488
> >Category:       bin
> >Synopsis:       Compat patch: more(1) allowed filename to start with '+'
> >Originator:     Dmitry Sivachenko
> >Release:        FreeBSD 4.8-STABLE i386
> >Description:
> more(1), before it became an alias to less(1), allowed filename to start with
> '+'.  Thus, `more +abc` was legal request to view file '+abc'.
> less(1) treats leading '+' symbol specially.  The following patch
> restores old behaviour (only when invoked as 'more').
> >Fix:
> --- main.c.orig	Mon Apr 28 14:36:38 2003
> +++ main.c	Mon Apr 28 14:38:21 2003
> @@ -125,7 +125,8 @@
>  	if (s != NULL)
>  		scan_option(save(s));
>  
> -#define	isoptstring(s)	(((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0')
> +#define	isoptstring(s)	more_mode ? (((s)[0] == '-') && (s)[1] != '\0') : \
> +			(((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0')
>  	while (argc > 0 && (isoptstring(*argv) || isoptpending()))
>  	{
>  		s = *argv++;

Just a minor style/correctness comment: IMHO, the ?: conditional should
be enclosed in another set of parentheses.

Other than that, this looks great!

G'luck,
Peter

-- 
Peter Pentchev	roam@ringlet.net    roam@sbnd.net    roam@FreeBSD.org
PGP key:	http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint	FDBA FD79 C26F 3C51 C95E  DF9E ED18 B68D 1619 4553
The rest of this sentence is written in Thailand, on

On Tue, 29 Apr 2003, Peter Pentchev wrote:

>  On Mon, Apr 28, 2003 at 02:44:04PM +0400, Dmitry Sivachenko wrote:
>  > -#define	isoptstring(s)	(((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0')
>  > +#define	isoptstring(s)	more_mode ? (((s)[0] == '-') && (s)[1] != '\0') : \
>  > +			(((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0')

>  Just a minor style/correctness comment: IMHO, the ?: conditional should
>  be enclosed in another set of parentheses.

This is necessary for correctness (in macros that expand to an expression,
the expression should have outer parethenthses, but it is not what KNF
actually does for "?:" so it would be a style bug in most contexts.

Unnecessary parentheses in macros are larger style bugs than in most places
IMO, since macros need lots of parentheses for correctness and mixing
unecessary ones with necessary ones makes the necessary ones hard to
distinguish for human readers.

In the above, the original version has minimal necessary parentheses
including the outer ones, but the outer ones have mutated into unnecessary
ones around each term in the "?:".

KNF is more or less defined by what KNF code like Lite2's kern/*.c
does, not by what style(9) says.  In Lite2's kern/*.c, there are 51
lines with "?:".  Approx. 12 of these lines have unnecessary parentheses.
2 of these parenthesize the expression like in "x = (y ? u : v)".  9 of
them parenthesize the conditional like in "(x & y) ? u : v".  One
parenthesizes a double conditional "x ? y : (u ? v : w)".  Unnecessary
parentheses work worst for longer multiple conditionals like the latter.
E.g., the idiomatic ladder:

	a ? b : c ? d : e ? f : g ? h : i ? j : k

becomes:

	a ? b : (c ? d : (e ? f : (g ? h : (i ? j : k))))

where it is hard to unpeel the parentheses to see that nothing special is
being done with them.  This is like obfuscating "else if" ladders using
"else { if ... }".

Bruce

Comment 3 Xin LI 2007-06-21 10:27:36 UTC

Responsible Changed
From-To: freebsd-bugs->delphij

Take

Comment 4 dfilter service 2007-06-21 11:39:30 UTC

delphij     2007-06-21 10:39:24 UTC

  FreeBSD src repository

  Modified files:
    contrib/less         main.c 
  Log:
  Restore a historical behavior that +foo is considered as a
  filename by more(1).  The less(1) behavior is keep intact.
  
  PR:             bin/51488
  Prodded by:     demon
  Approved by:    re (hrs)
  
  Revision  Changes    Path
  1.8       +2 -1      src/contrib/less/main.c
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"

Comment 5 Xin LI 2007-06-21 11:45:24 UTC

State Changed
From-To: open->patched

A variant patch applied against -CURRENT.  MFC Reminder.

Comment 6 Xin LI 2007-07-17 08:16:38 UTC

State Changed
From-To: patched->closed

less v406 MFC'ed.