PATCH: printf builtin

PATCH: printf builtin

[Oliver Kiddle]

Ksh and bash both have a printf builtin. This adds one to zsh.

I'm inclined to think this should be in a loadable module - it isn't
that useful interactively but then it isn't particularly large either.
Any views on that? If it should be a module, should it be compiled in
by default for a static zsh?

There seems to be a bit of inconsistency over what the \c escape means.
In zsh it cancels a trailing newline but in most other things, it
cancels all following output.

I've not used zstrtol because it doesn't set errno to ERANGE for
overflows. POSIX requires the user to be notified of such errors.
Someone more familiar with the zsh source and with C should maybe have
a quick look through: I'm not sure whether there shouldn't be a free()
or two in there and if so which one. I intend to do a bit more testing
and tidying on it tomorrow anyway. We don't seem to have any tests of
builtins so where in the tests should I stick tests for this?

I have included the bash and ksh %b and %q extensions to the format
which are like %s but interpret posix escapes and shell quote the
argument respectively. Note that you can't specify a width or precision
with %b because I wanted to allow \0 to be used to output a null
character and printf(3) would stop at the null.

I've not included ksh's %H (HTML quoting) or %R/%P (regex/glob
conversion) because I don't think they have any place being there. ksh
also has %(...)T which is like date(1). And, I've not worked out what
ksh's %Z does.

Let me know if you have any other ideas for extensions or any thoughts
on whether I should add the following:

    Support for %n would be easily doable taking the argument as the
    name of a parameter.

    Allow the argument to be specified by writing `%m$' in place of '%'
    so '%3$s' will print the third argument. IRIX's printf(1) does
    this. It would involve disabling the feature of reusing the format
    for remaining arguments to avoid infinite loops.

Oliver

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.53
diff -u -r1.53 builtin.c
--- Src/builtin.c	2001/09/18 17:50:26	1.53
+++ Src/builtin.c	2001/09/20 18:49:33
@@ -92,6 +92,7 @@
 
     BUILTIN("popd", 0, bin_cd, 0, 2, BIN_POPD, NULL, NULL),
     BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, "RDPbnrslzNu0123456789pioOcm-", NULL),
+    BUILTIN("printf", 0, bin_printf, 1, -1, 0, NULL, NULL),
     BUILTIN("pushd", 0, bin_cd, 0, 2, BIN_PUSHD, NULL, NULL),
     BUILTIN("pushln", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
     BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL),
@@ -3051,6 +3052,176 @@
     if (fout != stdout)
 	fclose(fout);
     return 0;
+}
+
+/* printf */
+
+/**/
+int
+bin_printf(char *name, char **args, char *ops, int func)
+{
+    int len, nnl, width, prec, type, ret = 0;
+    char *start, *endptr, *c, *fmt = getkeystring(*args, &len, 0, &nnl);
+    char **first = ++args;
+    char save;
+    union {
+	zlong l;
+	double d;
+	unsigned long u;
+	char *s;
+	int c;
+    } val;
+    
+    do {
+
+        for (c = fmt;c-fmt < len;c++) {
+	    type = prec = width = -1;
+
+	    if (*c != '%') {
+        	putchar(*c);
+		continue;
+	    }
+
+	    start = c++;
+	    if (*c == '%') {
+		putchar('%');
+		continue;
+	    }
+	    
+	    if (strchr("+- #", *c)) c++;
+	    
+	    if (*c == '*') {
+		width = (*args) ? strtoul(*args++, NULL, 0) : 0;
+		c++;
+	    } else {
+		while (idigit(*c)) c++;
+	    }
+	    
+	    if (*c == '.') {
+		c++;
+		if (*c == '*') {
+		    prec = (*args) ? strtoul(*args++, NULL, 0) : 0;
+		    c++;
+	    	} else {
+		    while (idigit(*c)) c++;
+		}
+	    }
+	    
+	    if (*c == 'l' || *c == 'L' || *c == 'h') c++;
+    	    
+	    switch (*c) {
+	    case 'c':
+		if (*args) {
+		    if (**args == Meta)
+		        val.c = (*args)[1] ^ 32;
+		    else
+		        val.c = **args;
+		    args++;
+		} else
+		    val.c = 0;
+		break;
+	    case 's':
+		if (*args)
+		    val.s = unmetafy(*args++, NULL);
+		else
+		    continue;
+		break;
+	    case 'b':
+		if (*args) {
+		    int l;
+		    char *b = getkeystring(*args++, &l, 0, &nnl);
+		    fwrite(b, l, 1, stdout);
+		}
+		continue;
+	    case 'q':
+		if (*args) {
+		    *c = 's';
+		    val.s = bslashquote(unmetafy(*args++, NULL), NULL, 0);
+		} else
+		    continue;
+		break;
+    	    case 'd':
+	    case 'i':
+	    	type=1;
+		break;
+    	    case 'e':
+	    case 'E':
+	    case 'f':
+	    case 'g':
+	    case 'G':
+	    	type=2;
+		break;
+	    case 'o':
+	    case 'u':
+	    case 'x':
+	    case 'X':
+		type=3;
+		break;
+	    default:
+		if (*c) {
+		    save = c[1];
+		    c[1] = '\0';
+		}
+	    	zerrnam(name, "%s: invalid directive", start, 0);
+	    	if (*c) c[1] = save;
+	    	ret = 1;
+		continue;
+	    }
+	    
+	    if (type > 0) {
+		if (*args && (**args == '\'' || **args == '"' )) {
+		    if (type == 2)
+		    	val.d = (*args)[1];
+		    else
+		    	val.l = (*args)[1];
+		    args++;
+		} else {
+	    	    switch (type) {
+		    case 1:
+			val.l = (*args) ? strtol(*args, &endptr, 0) : 0;
+			break;
+    	    	    case 2:
+			val.d = (*args) ? strtod(*args, &endptr) : 0;
+			break;
+		    case 3:
+			val.u = (*args) ? strtoul(*args, &endptr, 0) : 0;
+		    }
+		    if (*args) {
+			if (errno == ERANGE) {
+			    zerrnam(name, "`%s' arithmetic overflow", *args, 0);
+			    ret = 1;
+			} else if (**args && endptr == *args) {
+			    zerrnam(name, "`%s' expected numeric value", endptr, 0);
+			    ret = 1;
+			} else if (*endptr) {
+			    zerrnam(name, "`%s' not completely converted", *args, 0);
+			    ret = 1;
+			}
+			args++;
+		    }
+		}
+	    }
+	    save = c[1];
+	    c[1] = '\0';
+	    if (width >= 0) {
+		if (prec >= 0)
+		    printf(start, width, prec, val);
+		else
+		    printf(start, width, val);
+	    } else {
+		if (prec >= 0)
+		    printf(start, prec, val);
+		else
+		    printf(start, val);
+	    }
+	    c[1] = save;
+	
+	}
+
+    /* if there are remaining args, reuse format string */
+    } while (*args && args != first);
+
+    return ret;
 }
 
 /* shift builtin */

_____________________________________________________________________
This message has been checked for all known viruses by the 
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

Re: PATCH: printf builtin

[Bart Schaefer]

On Sep 20,  8:10pm, Oliver Kiddle wrote:
}
} Ksh and bash both have a printf builtin. This adds one to zsh.

Did you also notice that "print -f" in ksh is the same as printf?

This raises the question of whether it should be possible to combine
print -f with all the other print options, which would imply quite a
different implementation from what's in this patch, and would likely
preclude it from being a module.


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: printf builtin

[Oliver Kiddle]

Bart Schaefer wrote:
> 
> On Sep 20,  8:10pm, Oliver Kiddle wrote:
> }
> } Ksh and bash both have a printf builtin. This adds one to zsh.
> 
> Did you also notice that "print -f" in ksh is the same as printf?

Oh dear. I didn't notice that. print was one of the things which David
Korn was keen to have in the standard and without the -f, ksh's print
would be little more than echo. It's also quite a nice feature.

> This raises the question of whether it should be possible to combine
> print -f with all the other print options, which would imply quite a
> different implementation from what's in this patch, and would likely
> preclude it from being a module.

ksh allows -s, -p and -u to be combined with print -f. What you say
about this precluding it from being a module is certainly true. What I
think I'll do is commit this as it is for now. I also think this could
be safely committed to the stable branch so unless anyone says otherwise
I'll do that. My argument for this being that it doesn't touch any
existing functionality and just does something where we didn't do
anything useful before - so it is similar to us allowing new completion
functions to be added to 4.0.

I'll put the ksh print -f functionality on my todo list though. It makes
sense to combine the -b, -m, -s, -o, -O, -i, -u, -p, -z, -D and -P
options with -f. -r, -R, -n, -l, -N and -c would make no sense to
combine. -l and -N would be fairly redundant because it would be better
to use a format of "%s\n" or "%s\0" so they should perhaps then be
declared obsolete. -P (and maybe -D) is also perhaps slightly
unfortunate because a %P format specification might be better.

Oliver


_____________________________________________________________________
This message has been checked for all known viruses by the 
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

Re: PATCH: printf builtin

[Bart Schaefer]

On Sep 21, 11:58am, Oliver Kiddle wrote:
}
} I'll put the ksh print -f functionality on my todo list though. It makes
} sense to combine the -b, -m, -s, -o, -O, -i, -u, -p, -z, -D and -P
} options with -f. -r, -R, -n, -l, -N and -c would make no sense to
} combine. -l and -N would be fairly redundant because it would be better
} to use a format of "%s\n" or "%s\0" so they should perhaps then be
} declared obsolete. -P (and maybe -D) is also perhaps slightly
} unfortunate because a %P format specification might be better.

This all sounds fine except that I don't see any reason to declare any of
the existing options obsolete.

There is a bit of an issue with respect to combining with -s and -z, as
those at present imply -n, and -z also implies -r; the question is what
to do with trailing newlines if they appear in the format string.

(This'll be my last message until late Sunday, as I'm off to a wedding in
Yosemite National Park.)

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: printf builtin

[Peter Stephenson]

Oliver Kiddle wrote:
> Ksh and bash both have a printf builtin. This adds one to zsh.

Yes, this is a good idea.

> I'm inclined to think this should be in a loadable module - it isn't
> that useful interactively but then it isn't particularly large either.
> Any views on that? If it should be a module, should it be compiled in
> by default for a static zsh?

It depends how how large it's likely to grow.  We may end up implementing
more and more internally to avoid problems with differing versions of
printf (and pre-parsing printf text for passing on to printf in a slightly
different format is not something anyone would *want* to do anyway).  But
at the moment it's not a very large builtin --- plus there are precedents
for later moving builtins out to modules which are automatically linked in.

Some other comments: it's perennially annoying that printf here and
elsewhere doesn't add the newline in the way print does, so (unlike in
perl) they aren't natural companions.  For this and other reasons I'd quite
like a flag to ordinary print to turn on printf handling but keep all the
usually options --- although it does change the way arguments are
interpreted.  This allows us to make it useful without running into
compatibility problems with other implementations.

Also, I don't understand what's happening here.
  % printf "The answer is %d\x0c" 13
  The answer is -4265416
I can't seem to get any of the numeric formats to work.  In fact, I can't
get the string format to work either.  Maybe there's some portability
problem with the val union?  I haven't looked any closer.  Precision and
width don't make any difference; __packed__ doesn't help, nor would I
expect it to.

Also, it might be an idea, in common with other parts of the shell, to
perform a matheval on arguments which are going to be treated numerically,
then co-erce them to integer or floating point as needed.  (This should
also make things like `printf "%d" 1e3' work transparently --- if it
doesn't it's my fault and I haven't made the math code smart enough.)

Also, it's not good enough that zsh-style printf handling happens before
%-handling; ideally they should be concurrent, which is probably a lot more
work:
  % printf "\x25d\n" 32
  -4265416
--- same problem with %d as before, but the point is here the resulting `%'
should be treated literally.  Or it needs at least to be documented that
there are two distinct levels of processing, but I consider that a less
attractive solution, ultimately:  there are already quite enough levels of
print escape handling (c.f. `print "\\\\"') without making this a separate
one: either backslash handling, or percent handling, but not both on the
same piece of text.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: printf builtin

[Oliver Kiddle]

Bart Schaefer wrote:
> 
> There is a bit of an issue with respect to combining with -s and -z, as
> those at present imply -n, and -z also implies -r; the question is what
> to do with trailing newlines if they appear in the format string.

A difficult question that. To follow ksh's example with -s, only up to
the first newline would go in the history but ksh can't store multi-line
commands in its history in the first place. I think we should be putting
it in exactly as the user asked for with the trailing newline but I'm
not sure.

And, as far as I can tell -z doesn't imply -r.

Peter Stephenson wrote:
> 
> Some other comments: it's perennially annoying that printf here and
> elsewhere doesn't add the newline in the way print does, so (unlike in
> perl) they aren't natural companions.  For this and other reasons I'd quite
> like a flag to ordinary print to turn on printf handling but keep all the
> usually options --- although it does change the way arguments are
> interpreted.  This allows us to make it useful without running into
> compatibility problems with other implementations.

Well, if we do ksh's -f to turn on printf handling it would be easy to
then add another option to force adding a final newline. Any
suggestions on the choice of letter?

> Also, I don't understand what's happening here.
>   % printf "The answer is %d\x0c" 13
>   The answer is -4265416
> I can't seem to get any of the numeric formats to work.  In fact, I can't
> get the string format to work either.  Maybe there's some portability
> problem with the val union?  I haven't looked any closer.  Precision and
> width don't make any difference; __packed__ doesn't help, nor would I
> expect it to.

What platform are you getting that with? I suspect the val union too but
it'd be nice to check.

When I used that val union, I feared that it would have portability
problems but the alternative was to repeat the block of code with the
varying calls to printf about five times. Does anyone have any ideas how
to maybe get this working. I don't really have a clue how the special
case of varying arguments to printf is handled. Would it maybe be
possible to record the sizeof() for the part of val used and have some
sort of typecast fix the size of val when it is passed to printf?

> Also, it might be an idea, in common with other parts of the shell, to
> perform a matheval on arguments which are going to be treated numerically,
> then co-erce them to integer or floating point as needed.  (This should
> also make things like `printf "%d" 1e3' work transparently --- if it
> doesn't it's my fault and I haven't made the math code smart enough.)

That's not a bad idea. Presumably this would also allow the parameter to
be a variable name or a calculation too.

> Also, it's not good enough that zsh-style printf handling happens before
> %-handling; ideally they should be concurrent, which is probably a lot more
> work:
>   % printf "\x25d\n" 32
>   -4265416
> --- same problem with %d as before, but the point is here the resulting `%'
> should be treated literally.  Or it needs at least to be documented that

I did think about that. I was very suprised that printf(3) seems to
work this way. Bash and ksh also do but a couple of external printf(1)s
work - treating the resulting `%' literally. I agree with you that that
is the better behaviour though. getkeystring() will need changes for
this so I'll leave this until I merge it into bin_print() and I'll deal
with the \c situation at the same time.

Cheers

Oliver

_____________________________________________________________________
This message has been checked for all known viruses by the 
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

Re: PATCH: printf builtin

[Peter Stephenson]

Oliver Kiddle wrote:
> Well, if we do ksh's -f to turn on printf handling it would be easy to
> then add another option to force adding a final newline.

If it's only optional, there's not really any advantage in an option over
an additional "\n" at the end.  The point was simply to try to keep normal
print behaviour where possible.

> >   % printf "The answer is %d\x0c" 13
> >   The answer is -4265416
>
> What platform are you getting that with? I suspect the val union too but
> it'd be nice to check.

Sorry, it's Solaris, 2.6 to 2.8.  I may get round to looking at it on
Monday but the combined effect of staring at a screen and the bright lights
in our new offices is putting me off at the moment.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

print -z (Re: PATCH: printf builtin)

[Bart Schaefer]

On Sep 21,  5:13pm, Oliver Kiddle wrote:
}
} Bart Schaefer wrote:
} > 
} > There is a bit of an issue with respect to combining with -s and -z, as
} > those at present imply -n, and -z also implies -r
} 
} And, as far as I can tell -z doesn't imply -r.

Hmm, the doc says:

    -z
          Push the arguments onto the editing buffer stack, separated
          by spaces; no escape sequences are recognized.
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The above is correct for zsh 2.4, but not for zsh 3.0 or later.  I'm
not sure exactly when it became wrong.  Does anyone remember whether
the change in behavior was intentional (and the doc simply forgotten),
or was this an accident and we should repair it to match the doc?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: print -z (Re: PATCH: printf builtin)

[Peter Stephenson]

Bart Schaefer wrote:
>     -z
>           Push the arguments onto the editing buffer stack, separated
>           by spaces; no escape sequences are recognized.
>                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> The above is correct for zsh 2.4, but not for zsh 3.0 or later.  I'm
> not sure exactly when it became wrong.  Does anyone remember whether
> the change in behavior was intentional (and the doc simply forgotten),
> or was this an accident and we should repair it to match the doc?

I'm not aware of any intentional change like that.

Arguably it's useful, since you can use print -zr in this case whereas
there's no way of doing the opposite.  But for most people it's probably
confusing.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: printf builtin

[Peter Stephenson]

Peter Stephenson wrote:
> Also, I don't understand what's happening here.
>   % printf "The answer is %d\x0c" 13
>   The answer is -4265416
> I can't seem to get any of the numeric formats to work.  In fact, I can't
> get the string format to work either.  Maybe there's some portability
> problem with the val union?

It does seem to be that, since everything looks OK up to the call to
printf.  As a test, I got it to work with a nasty hack using a void * and
separate variables for different types.

One problem I can see is that val has an element `zlong'.  Unfortunately on
32-bit machines with the default --enable-lfs, whether or not actual large
file support is available, that's usually a long long, i.e. 64-bits.  This
will certainly confuse printf with a simple `%d' --- Solaris supports
`%lld' but that's unlikely to be completely portable.  I'm afraid that's
another argument for handling the printf codes internally.  However, if we
use matheval(), the only basic numeric types we need to handle are doubles
and zlongs, plus some casting to unsigned: check Src/Builtins/rlimits.c for
some existing support for outputting long long or quad integers via printf.
(I've never actually seen a quad myself.)

However, there seems to be another problem, since changing that to long or
int (which are supposed to be the same here but I'm paranoid --- although
strictly we should enforce %ld if we pass on a long) doesn't help.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

PATCH (docs): Re: print -z (Re: PATCH: printf builtin)

[Bart Schaefer]

On Sep 24, 10:18am, Peter Stephenson wrote:
} Subject: Re: print -z (Re: PATCH: printf builtin)
}
} Bart Schaefer wrote:
} >     -z
} >           Push the arguments onto the editing buffer stack, separated
} >           by spaces; no escape sequences are recognized.
} >                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
} > 
} > The above is correct for zsh 2.4, but not for zsh 3.0 or later.  I'm
} > not sure exactly when it became wrong.  Does anyone remember whether
} > the change in behavior was intentional (and the doc simply forgotten),
} > or was this an accident and we should repair it to match the doc?
} 
} I'm not aware of any intentional change like that.

It looks like it was intentional:

  http://www.zsh.org/cgi-bin/mla/redirect?WORKERNUMBER=214

} Arguably it's useful, since you can use print -zr in this case whereas
} there's no way of doing the opposite.  But for most people it's probably
} confusing.

Heh.  I think "most people" don't use `print -z' at all, or we'd have
noticed this before 6 years had passed.

Index: Doc/Zsh/builtins.yo
===================================================================
diff -c -r1.9 builtins.yo
--- Doc/Zsh/builtins.yo	2001/09/15 19:16:23	1.9
+++ Doc/Zsh/builtins.yo	2001/09/24 16:32:38
@@ -702,8 +702,7 @@
 Print the arguments to the input of the coprocess.
 )
 item(tt(-z))(
-Push the arguments onto the editing buffer stack, separated by spaces;
-no escape sequences are recognized.
+Push the arguments onto the editing buffer stack, separated by spaces.
 )
 item(tt(-D))(
 Treat the arguments as directory names, replacing prefixes with tt(~)


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: printf builtin

[Oliver Kiddle]

Peter Stephenson wrote:
> 
> It does seem to be that, since everything looks OK up to the call to
> printf.  As a test, I got it to work with a nasty hack using a void * and
> separate variables for different types.

Could you please send me this `nasty hack'. I'd prefer to get something
which works and has all the features required by POSIX, commit it and
then worry about the many possible improvements as and when I have time
and feel inclined to do them.

> One problem I can see is that val has an element `zlong'.  Unfortunately on
> 32-bit machines with the default --enable-lfs, whether or not actual large
> file support is available, that's usually a long long, i.e. 64-bits.  This
> will certainly confuse printf with a simple `%d' --- Solaris supports

Right. I should have just used an int then it seems.

> `%lld' but that's unlikely to be completely portable.  I'm afraid that's
> another argument for handling the printf codes internally.  However, if we

I don't dispute that handling printf codes internally would be a good
thing to be doing eventually but my initial aim was just to have the
POSIX functionality and it shouldn't be necessary for that.

Judging by certain aspects of their behaviour I suspect other printf(1)
implementations are using the underlying printf(3) (with the possible
exception of bash).

> use matheval(), the only basic numeric types we need to handle are doubles
> and zlongs, plus some casting to unsigned: check Src/Builtins/rlimits.c for

Would you then suggest that in the final implementation, `%d' would
format a zlong and that we wouldn't bother with any size modifiers (or
just ignore them). It would seem sensible because C only needs size
modifiers because the parameters can be of various types.

> However, there seems to be another problem, since changing that to long or
> int (which are supposed to be the same here but I'm paranoid --- although
> strictly we should enforce %ld if we pass on a long) doesn't help.

Sorry. Are you saying that it still doesn't work with an int? That
wouldn't make sense.

Oliver

_____________________________________________________________________
This message has been checked for all known viruses by the 
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

Re: PATCH: printf builtin

[Peter Stephenson]

Oliver Kiddle wrote:
> Peter Stephenson wrote:
> > It does seem to be that, since everything looks OK up to the call to
> > printf.  As a test, I got it to work with a nasty hack using a void * and
> > separate variables for different types.
> 
> Could you please send me this `nasty hack'. I'd prefer to get something
> which works and has all the features required by POSIX, commit it and
> then worry about the many possible improvements as and when I have time
> and feel inclined to do them.

I've stuck the revised printf below, but I'm sure you can do a lot better
than that.  It's not a patch because I don't particularly want people to
apply it.

> Would you then suggest that in the final implementation, `%d' would
> format a zlong and that we wouldn't bother with any size modifiers (or
> just ignore them).

Yes, absolutely.  We should use integers and doubles in the internal format
only, and just ignore modifiers --- I don't see any possible effect beyond
confusion in trying to support different sizes.  We could in principle cast
to the required size, but this means that a simple %d won't work with the
internal type, which is silly.

The only possible exception is how we handle unsigned values.  printf will
normally take the same bits and print them as if they were an unsigned
value.  I'm unsure whether this is useful, but don't see another sensible
interpretation of `printf %ud -1'.

> > However, there seems to be another problem, since changing that to long or
> > int (which are supposed to be the same here but I'm paranoid --- although
> > strictly we should enforce %ld if we pass on a long) doesn't help.
> 
> Sorry. Are you saying that it still doesn't work with an int? That
> wouldn't make sense.

No, this was still with the union; there's something wrong there apart from
the size of the integer variable.  It doesn't affect the version below.


/* printf */

/**/
int
bin_printf(char *name, char **args, char *ops, int func)
{
    int len, nnl, width, prec, type, ret = 0;
    char *start, *endptr, *c, *fmt = getkeystring(*args, &len, 0, &nnl);
    char **first = ++args;
    char save;

    /* instead of union */
    char *stringval;
    int intval;
    double doubleval;
    unsigned int uintval;
    void **val;
    
    do {

        for (c = fmt;c-fmt < len;c++) {
	    type = prec = width = -1;

	    if (*c != '%') {
        	putchar(*c);
		continue;
	    }

	    start = c++;
	    if (*c == '%') {
		putchar('%');
		continue;
	    }
	    
	    if (strchr("+- #", *c)) c++;
	    
	    if (*c == '*') {
		width = (*args) ? strtoul(*args++, NULL, 0) : 0;
		c++;
	    } else {
		while (idigit(*c)) c++;
	    }
	    
	    if (*c == '.') {
		c++;
		if (*c == '*') {
		    prec = (*args) ? strtoul(*args++, NULL, 0) : 0;
		    c++;
	    	} else {
		    while (idigit(*c)) c++;
		}
	    }
	    
	    if (*c == 'l' || *c == 'L' || *c == 'h') c++;
    	    
	    switch (*c) {
	    case 'c':
		type = 1;
		if (*args) {
		    if (**args == Meta)
		        intval = (*args)[1] ^ 32;
		    else
		        intval = **args;
		    args++;
		} else
		    intval = 0;
		break;
	    case 's':
		if (*args)
		    stringval = unmetafy(*args++, NULL);
		else
		    continue;
		break;
	    case 'b':
		if (*args) {
		    int l;
		    char *b = getkeystring(*args++, &l, 0, &nnl);
		    fwrite(b, l, 1, stdout);
		}
		continue;
	    case 'q':
		if (*args) {
		    *c = 's';
		    stringval = bslashquote(unmetafy(*args++, NULL), NULL, 0);
		} else
		    continue;
		break;
    	    case 'd':
	    case 'i':
	    	type=1;
		break;
    	    case 'e':
	    case 'E':
	    case 'f':
	    case 'g':
	    case 'G':
	    	type=2;
		break;
	    case 'o':
	    case 'u':
	    case 'x':
	    case 'X':
		type=3;
		break;
	    default:
		if (*c) {
		    save = c[1];
		    c[1] = '\0';
		}
	    	zerrnam(name, "%s: invalid directive", start, 0);
	    	if (*c) c[1] = save;
	    	ret = 1;
		continue;
	    }
	    
	    if (type > 0) {
		if (*args && (**args == '\'' || **args == '"' )) {
		    if (type == 2)
		    	doubleval = (*args)[1];
		    else
		    	intval = (*args)[1];
		    args++;
		} else {
	    	    switch (type) {
		    case 1:
			intval = (*args) ? strtol(*args, &endptr, 0) : 0;
			break;
    	    	    case 2:
			doubleval = (*args) ? strtod(*args, &endptr) : 0;
			break;
		    case 3:
		        uintval = (*args) ? strtoul(*args, &endptr, 0) : 0;
		    }
		    if (*args) {
			if (errno == ERANGE) {
			    zerrnam(name, "`%s' arithmetic overflow", *args, 0);
			    ret = 1;
			} else if (**args && endptr == *args) {
			    zerrnam(name, "`%s' expected numeric value", endptr, 0);
			    ret = 1;
			} else if (*endptr) {
			    zerrnam(name, "`%s' not completely converted", *args, 0);
			    ret = 1;
			}
			args++;
		    }
		}
	    }
	    save = c[1];
	    c[1] = '\0';
	    /* yuk */
	    switch (type) {
	    case 1:
		val = (void *) &intval;
		break;
	    case 2:
		val = (void *) &doubleval;
		break;
	    case 3:
		val = (void *) &uintval;
		break;
	    default:
		val = (void *) &stringval;
		break;
	    }
	    if (width >= 0) {
		if (prec >= 0)
		    printf(start, width, prec, *val);
		else
		    printf(start, width, *val);
	    } else {
		if (prec >= 0)
		    printf(start, prec, *val);
		else
		    printf(start, *val);
	    }
	    c[1] = save;
	
	}

    /* if there are remaining args, reuse format string */
    } while (*args && args != first);

    return ret;
}

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: printf builtin

[Oliver Kiddle]

Peter Stephenson wrote:

> It does seem to be that, since everything looks OK up to the call to
> printf.  As a test, I got it to work with a nasty hack using a void * and
> separate variables for different types.

The void * hack was also not portable (doubles on Linux didn't work).
Instead, I've used a macro so the duplication is avoided in the source
if not in the compiled code. Should this macro be moved elsewhere in
the source at all? I'll commit this now but I should be around next
week so can fix any problems in a more timely manner than with getting
this last little bit done.

Oliver

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.54
diff -u -r1.54 builtin.c
--- Src/builtin.c	2001/09/24 10:12:51	1.54
+++ Src/builtin.c	2001/10/05 14:42:33
@@ -92,6 +92,7 @@
 
     BUILTIN("popd", 0, bin_cd, 0, 2, BIN_POPD, NULL, NULL),
     BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, "RDPbnrslzNu0123456789pioOcm-", NULL),
+    BUILTIN("printf", 0, bin_printf, 1, -1, 0, NULL, NULL),
     BUILTIN("pushd", 0, bin_cd, 0, 2, BIN_PUSHD, NULL, NULL),
     BUILTIN("pushln", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
     BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL),
@@ -3051,6 +3052,179 @@
     if (fout != stdout)
 	fclose(fout);
     return 0;
+}
+
+/* printf */
+
+#define print_val(VAL) \
+  if (width >= 0) { \
+      if (prec >= 0) \
+	  printf(start, width, prec, VAL); \
+      else \
+	  printf(start, width, VAL); \
+  } else { \
+      if (prec >= 0) \
+	  printf(start, prec, VAL); \
+      else \
+	  printf(start, VAL); \
+  }
+
+/**/
+int
+bin_printf(char *name, char **args, char *ops, int func)
+{
+    int len, nnl, width, prec, type, ret = 0;
+    char *start, *endptr, *c, *fmt = getkeystring(*args, &len, 0, &nnl);
+    char **first = ++args, nullstr = '\0', save = '\0';
+
+    double doubleval;
+    int intval;
+    unsigned int uintval;
+    char *stringval;
+
+    do {
+
+	for (c = fmt;c-fmt < len;c++) {
+	    type = prec = width = -1;
+
+	    if (*c != '%') {
+		putchar(*c);
+		continue;
+	    }
+
+	    start = c++;
+	    if (*c == '%') {
+		putchar('%');
+		continue;
+	    }
+
+	    if (strchr("+- #", *c)) c++;
+
+	    if (*c == '*') {
+		width = (*args) ? strtoul(*args++, NULL, 0) : 0;
+		c++;
+	    } else {
+		while (idigit(*c)) c++;
+	    }
+
+	    if (*c == '.') {
+		c++;
+		if (*c == '*') {
+		    prec = (*args) ? strtoul(*args++, NULL, 0) : 0;
+		    c++;
+		} else {
+		    while (idigit(*c)) c++;
+		}
+	    }
+
+	    if (*c == 'l' || *c == 'L' || *c == 'h') c++;
+
+	    if (*c) {
+		save = c[1];
+		c[1] = '\0';
+	    }
+	    switch (*c) {
+	    case 'c':
+		if (*args) {
+		    if (**args == Meta)
+			intval = (*args)[1] ^ 32;
+		    else
+			intval = **args;
+		    args++;
+		} else
+		    intval = 0;
+		print_val(intval);
+		break;
+	    case 's':
+		if (*args)
+		    stringval = unmetafy(*args++, NULL);
+		else
+		    stringval = &nullstr;
+		print_val(stringval);
+		break;
+	    case 'b':
+		if (*args) {
+		    int l;
+		    char *b = getkeystring(*args++, &l, 0, &nnl);
+		    fwrite(b, l, 1, stdout);
+		}
+		continue;
+	    case 'q':
+		if (*args)
+		    stringval = bslashquote(unmetafy(*args++, NULL), NULL, 0);
+		else
+		    stringval = &nullstr;
+		*c = 's';
+		print_val(stringval);
+		break;
+	    case 'd':
+	    case 'i':
+		type=1;
+		break;
+	    case 'e':
+	    case 'E':
+	    case 'f':
+	    case 'g':
+	    case 'G':
+		type=2;
+		break;
+	    case 'o':
+	    case 'u':
+	    case 'x':
+	    case 'X':
+		type=3;
+		break;
+	    default:
+		zerrnam(name, "%s: invalid directive", start, 0);
+		ret = 1;
+	    }
+
+	    if (type > 0) {
+		if (*args && (**args == '\'' || **args == '"' )) {
+		    if (type == 2) {
+			doubleval = (*args)[1];
+			print_val(doubleval);
+		    } else {
+		    	intval = (*args)[1];
+			print_val(intval);
+		    }
+		    args++;
+		} else {
+	    	    switch (type) {
+		    case 1:
+			intval = (*args) ? strtol(*args, &endptr, 0) : 0;
+			print_val(intval);
+			break;
+		    case 2:
+			doubleval = (*args) ? strtod(*args, &endptr) : 0;
+			print_val(doubleval);
+			break;
+		    case 3:
+			uintval = (*args) ? strtoul(*args, &endptr, 0) : 0;
+			print_val(uintval);
+		    }
+		    if (*args) {
+			if (errno == ERANGE) {
+			    zerrnam(name, "`%s' arithmetic overflow", *args, 0);
+			    ret = 1;
+			} else if (**args && endptr == *args) {
+			    zerrnam(name, "`%s' expected numeric value", endptr, 0);
+			    ret = 1;
+			} else if (*endptr) {
+			    zerrnam(name, "`%s' not completely converted", *args, 0);
+			    ret = 1;
+			}
+			args++;
+		    }
+		}
+	    }
+	    if (*c) c[1] = save;
+	}
+
+    /* if there are remaining args, reuse format string */
+    } while (*args && args != first);
+
+    return ret;
 }
 
 /* shift builtin */

_____________________________________________________________________
This message has been checked for all known viruses by the 
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

Re: PATCH: printf builtin

[Peter Stephenson]

Oliver Kiddle wrote:
> +#define print_val(VAL) \
> +  if (width >= 0) { \
> +      if (prec >= 0) \
> +	  printf(start, width, prec, VAL); \
> +      else \
> +	  printf(start, width, VAL); \
> +  } else { \
> +      if (prec >= 0) \
> +	  printf(start, prec, VAL); \
> +      else \
> +	  printf(start, VAL); \
> +  }

I was wondering if we could do something better with vfprintf, but actually
it's not obvious we can.

In the long run, upgrading convbase, convfloat and padding for parameter
strings probably provides the neatest solution.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************