PATCH: Slightly improve printf %s

PATCH: Slightly improve printf %s

[Thorsten Dahlheimer]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

This change makes it possible to print strings containing NULs with
printf '%s', by sharing the code for the %b specifier.
After I made this change, the test suite uncovered a bug (which I
introduced) in the handling of %b (and now %s) when no argument is
left, so the patch includes a fix for that, too.

Regards,
Thorsten Dahlheimer

[-- Attachment #2: printf_s.patch --]
[-- Type: application/octet-stream, Size: 1024 bytes --]

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.143
diff -u -p -r1.143 builtin.c
--- Src/builtin.c	1 Aug 2005 22:20:36 -0000	1.143
+++ Src/builtin.c	8 Aug 2005 15:31:32 -0000
@@ -3752,14 +3752,18 @@ bin_print(char *name, char **args, Optio
 		print_val(intval);
 		break;
 	    case 's':
-		stringval = curarg ? curarg : &nullstr;
-		print_val(stringval);
-		break;
 	    case 'b':
 		if (curarg) {
+		    char *b;
 		    int l;
-		    char *b = getkeystring(metafy(curarg, curlen, META_USEHEAP), &l,
-					   OPT_ISSET(ops,'b') ? 2 : 0, &nnl);
+		    if (*c == 'b')
+			b = getkeystring(metafy(curarg, curlen, META_USEHEAP), &l,
+					 OPT_ISSET(ops,'b') ? 2 : 0, &nnl);
+		    else {
+			b = curarg;
+			l = curlen;
+			nnl = 0;
+		    }
 		    /* handle width/precision here and use fwrite so that
 		     * nul characters can be output */
 		    if (prec >= 0 && prec < l) l = prec;

Re: PATCH: Slightly improve printf %s

[Wayne Davison]

On Mon, Aug 08, 2005 at 08:21:28PM +0200, Thorsten Dahlheimer wrote:
> After I made this change, the test suite uncovered a bug (which I
> introduced) in the handling of %b (and now %s) when no argument is
> left, so the patch includes a fix for that, too.

I didn't see that fix in the patch.  I assume you're talking about
this line (which is executed when the arg is missing):

        count += fprintf(fout, "%*c", width, ' ');

... since that would output a single space, even when "width" is 0.  I
changed the "else" that is in front of that line to "else if (width)",
and it fixes the problem (I could have also changed the line to use
fprintf(fout, "%*s", width, ""), but the former seems a litle more
optimal).

As for the change to how %s is handled, can you show me an example where
this is needed?  In my simple tests, if a variable has a literal null in
it, using either %b or %s to print it out with printf truncates the
variable at the null (and, of course, a backslash-zero can be passed to
%b to get a real null, but %s doesn't evaluate backslashes).

..wayne..

Re: PATCH: Slightly improve printf %s

[Thorsten Dahlheimer]

Wayne Davison wrote:
> On Mon, Aug 08, 2005 at 08:21:28PM +0200, Thorsten Dahlheimer wrote:
> > After I made this change, the test suite uncovered a bug (which I
> > introduced) in the handling of %b (and now %s) when no argument is
> > left, so the patch includes a fix for that, too.
> 
> I didn't see that fix in the patch.  I assume you're talking about
> this line (which is executed when the arg is missing):
> 
>         count += fprintf(fout, "%*c", width, ' ');
> 
> ... since that would output a single space, even when "width" is 0.  I
> changed the "else" that is in front of that line to "else if (width)",
> and it fixes the problem (I could have also changed the line to use
> fprintf(fout, "%*s", width, ""), but the former seems a litle more
> optimal).

Sorry, I attached the wrong diff (the one that I made before I discovered
that bug).  Anyway, the fix I made is exactly the one you've described.

> As for the change to how %s is handled, can you show me an example where
> this is needed?  In my simple tests, if a variable has a literal null in
> it, using either %b or %s to print it out with printf truncates the
> variable at the null (and, of course, a backslash-zero can be passed to
> %b to get a real null, but %s doesn't evaluate backslashes).

I don't know if it's really needed, but it would certainly be more
consistent.  Currently you get:

    % s=$'a\0b'

    % print -n $s |od -tx1
    0000000 61 00 62
    0000003

    % printf $s |od -tx1  
    0000000 61 00 62
    0000003

    % printf '%b' $s |od -tx1
    0000000 61 00 62
    0000003

    % printf '%s' $s |od -tx1
    0000000 61
    0000001

(Note that the behaviour of %b in this case changed as a consequence
of zsh-workers/21552, item 4.)  Given that the first three variants
treat the nul just like any other character, and the same is true
for the rest of zsh, I find the behaviour of %s a bit surprising.
I think it's conceivable that someone processes strings containing
nuls (say, captured output from 'find ... -print0' or the like) and
finally writes them out with print; this all works well with zsh.
Then they change the output command from 'print -nr -- "$s"' to
'printf "...%s..." ... "$s" ...' and get bitten.

Regards,
Thorsten Dahlheimer

Re: PATCH: Slightly improve printf %s

[Wayne Davison]

On Tue, Aug 09, 2005 at 04:03:32PM +0200, Thorsten Dahlheimer wrote:
> (Note that the behaviour of %b in this case changed as a consequence
> of zsh-workers/21552, item 4.)

Ah, yes -- I apparently wasn't testing the latest CVS version like I
thought I was.  Given that, it certainly makes sense to me to fix %s
too.

..wayne..