Bug 208554

Summary: usr.bin/sed :sed functions 'i' and 'a' discard leading white space
Product: Base System Reporter: Pedro F. Giffuni <pfg>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: bdrewery, emaste
Priority: --- Keywords: patch
Version: CURRENT   
Hardware: Any   
OS: Any   
URL: http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=49872
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213474
Attachments:
Description Flags
Patch from NetBSD none

Description Pedro F. Giffuni freebsd_committer freebsd_triage 2016-04-05 16:27:07 UTC
Created attachment 169011 [details]
Patch from NetBSD

sed functions 'a' and 'i' discard leading white space of the inserted/appended line(s) of text.
This comes from NetBSD's PR/49872

I have confirmed their test on FreeBSD:

(old sed)
pfg@gorilon:~/test % echo | sed -f test.sed | vis -l
test 2  \$
\$
test 1  \$
pfg@gorilon:~/test % vis -l < test.sed
a\134\$
   test 1  \$
 i\134\$
   test 2  \$
\$
(new sed)
pfg@gorilon:~/test % echo | sed -f test.sed | vis -l
   test 2  \$
\$
   test 1  \$
______
New behavior would be similar to GNU sed.
Comment 1 Bryan Drewery freebsd_committer freebsd_triage 2016-04-05 22:19:04 UTC
I think this makes sense as otherwise you cannot control whitespace in the
line you are adding.  Reading opengroup there's no mention of eating
whitespace on these commands either.

*However*, we've had this particular behavior forever, since at least r1591.

The removal in NetBSD follows it accidentally being added back in 2014.
The real removal came in 2004:

> commit 39dd7ae9eebb61b8b83c15e441faa17593a5ec8d
> Author: matt <matt>
> Date:   Wed Nov 17 22:17:54 2004 +0000
>
>     When adding text due to an a, c, or i command, don't eat the space(s) at
>     the beginning of the lines since the addition is supposed to be "verbatim".
>     Fix a comment for compile_text indicating it's also used for the 'c' command.
>
> diff --git a/usr.bin/sed/compile.c b/usr.bin/sed/compile.c
> index 4ae0e53..4a185de 100644
> --- a/usr.bin/sed/compile.c
> +++ b/usr.bin/sed/compile.c
> @@ -1,4 +1,4 @@
> -/*	$NetBSD: compile.c,v 1.30 2004/07/09 23:43:07 enami Exp $	*/
> +/*	$NetBSD: compile.c,v 1.31 2004/11/17 22:17:54 matt Exp $	*/
>  
>  /*-
>   * Copyright (c) 1992, 1993
> @@ -72,7 +72,7 @@
>  #if 0
>  static char sccsid[] = "@(#)compile.c	8.2 (Berkeley) 4/28/95";
>  #else
> -__RCSID("$NetBSD: compile.c,v 1.30 2004/07/09 23:43:07 enami Exp $");
> +__RCSID("$NetBSD: compile.c,v 1.31 2004/11/17 22:17:54 matt Exp $");
>  #endif
>  #endif /* not lint */
>  
> @@ -666,7 +666,7 @@ compile_tr(char *p, char **transtab)
>  }
>  
>  /*
> - * Compile the text following an a or i command.
> + * Compile the text following an a, c, or i command.
>   */
>  static char *
>  compile_text(void)
> @@ -681,7 +681,6 @@ compile_text(void)
>  	while (cu_fgets(lbuf, sizeof(lbuf))) {
>  		op = s = text + size;
>  		p = lbuf;
> -		EATSPACE();
>  		for (; *p; p++) {
>  			if (*p == '\\')
>  				p++;


I support removing it, especially since GNU and NetBSD do so as well and
it gives more control to the user.
Comment 2 commit-hook freebsd_committer freebsd_triage 2016-04-06 00:56:41 UTC
A commit references this bug:

Author: pfg
Date: Wed Apr  6 00:55:40 UTC 2016
New revision: 297602
URL: https://svnweb.freebsd.org/changeset/base/297602

Log:
  Fix sed functions 'i' and 'a' from discarding leading white space.

  This appears to be implementation dependent but convenient and makes
  our sed behave more like GNU sed.

  Given that it is not the historic behavior, bump FreeBSD_version
  should userland/ports somehow depend on it.

  Obtained from:	NetBSD (bin/49872)

  Reviewed by:	bdrewery
  PR:		208554
  Merge after:	NEVER

Changes:
  head/sys/sys/param.h
  head/usr.bin/sed/compile.c
Comment 3 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-04-06 01:21:13 UTC
(In reply to Bryan Drewery from comment #1)

Thank you for the feedback.

I have been noticing some of the ports that we build with GNU sed can now be with the sed in current so this change helps improve the compatibility without hurting standards compliance.

While I don't see a situation where we may want the previous behavior, I don't think it is convenient to merge it to stable.