Subject: Re: 64 bit shell arithmetic
To: None <tech-userlevel@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-userlevel
Date: 03/24/2007 17:27:20
In article <20070324132353.GC8664@apb-laptoy.apb.alt.za>,
Alan Barrett  <apb@cequrux.com> wrote:
>On Fri, 23 Mar 2007, Markus Mayer wrote:
>> It looks like I discovered a problem with shell arithmetic with large
>> number (numbers larger than "signed long").
>
>Here's a patch to make it use intmax_t.  I'd appreciate review from
>people who understand yacc better than I do.
>
>--apb (Alan Barrett)
>
>Index: src/bin/sh/arith.y
>===================================================================
>--- arith.y	17 Sep 2003 17:33:36 -0000	1.17
>+++ arith.y	24 Mar 2007 12:46:14 -0000
>@@ -49,6 +49,10 @@
> #include "output.h"
> #include "memalloc.h"
> 
>+typedef intmax_t YYSTYPE;
>+#define YYSTYPE YYSTYPE
>+
>+intmax_t arith_result;
> const char *arith_buf, *arith_startbuf;
> 
> void yyerror(const char *);
>@@ -74,7 +78,12 @@
> %%
> 
> exp:	expr {
>-			return ($1);
>+			/*
>+			 * yyparse() returns int, so we have to save
>+			 * the desired result elsewhere.
>+			 */
>+			arith_result = $1;
>+			return 0;
> 		}
> 	;
> 
>@@ -113,16 +122,17 @@
> 	| ARITH_NUM
> 	;
> %%
>-int
>+intmax_t
> arith(s)
> 	const char *s;
> {
>-	long result;
>+	intmax_t result;
> 
> 	arith_buf = arith_startbuf = s;
> 
> 	INTOFF;
>-	result = yyparse();
>+	(void) yyparse();
>+	result = arith_result;
> 	arith_lex_reset();	/* reprime lex */
> 	INTON;
> 
>@@ -141,7 +151,7 @@
> 	const char *p;
> 	char *concat;
> 	char **ap;
>-	long i;
>+	intmax_t i;
> 
> 	if (argc > 1) {
> 		p = argv[1];
>@@ -164,9 +174,10 @@
> 	} else
> 		p = "";
> 
>-	i = arith(p);
>+	(void)arith(p);
>+	i = arith_result;
> 
>-	out1fmt("%ld\n", i);
>+	out1fmt("%"PRIdMAX"\n", i);
> 	return (! i);
> }
> 
>@@ -176,7 +187,7 @@
> main(argc, argv)
> 	char *argv[];
> {
>-	printf("%d\n", exp(argv[1]));
>+	printf("%"PRIdMAX"\n", exp(argv[1]));
> }
> error(s)
> 	char *s;
>Index: src/bin/sh/arith_lex.l
>===================================================================
>--- arith_lex.l	21 Mar 2005 22:37:09 -0000	1.13
>+++ arith_lex.l	24 Mar 2007 12:46:14 -0000
>@@ -48,8 +48,8 @@
> #include "expand.h"
> #include "var.h"
> 
>-extern int yylval;
>-extern char *arith_buf, *arith_startbuf;
>+extern intmax_t yylval;
>+extern const char *arith_buf, *arith_startbuf;
> #undef YY_INPUT
> #define YY_INPUT(buf,result,max) \
> 	result = (*buf = *arith_buf++) ? 1 : YY_NULL;
>@@ -58,12 +58,12 @@
> 
> %%
> [ \t\n]	{ ; }
>-0x[0-9a-fA-F]+	{ yylval = strtol(yytext, 0, 0); return(ARITH_NUM); }
>-0[0-7]*		{ yylval = strtol(yytext, 0, 0); return(ARITH_NUM); }
>-[1-9][0-9]*	{ yylval = strtol(yytext, 0, 0); return(ARITH_NUM); }
>+0x[0-9a-fA-F]+	{ yylval = strtoimax(yytext, 0, 0); return(ARITH_NUM); }
>+0[0-7]*		{ yylval = strtoimax(yytext, 0, 0); return(ARITH_NUM); }
>+[1-9][0-9]*	{ yylval = strtoimax(yytext, 0, 0); return(ARITH_NUM); }
> [A-Za-z_][A-Za-z_0-9]*	{ char *v = lookupvar(yytext);
> 			if (v) {
>-				yylval = strtol(v, &v, 0);
>+				yylval = strtoimax(v, &v, 0);
> 				if (*v == 0)
> 					return ARITH_NUM;
> 			}
>Index: src/bin/sh/expand.c
>===================================================================
>--- expand.c	24 Nov 2006 22:54:47 -0000	1.77
>+++ expand.c	24 Mar 2007 12:46:15 -0000
>@@ -352,7 +352,8 @@
> expari(int flag)
> {
> 	char *p, *start;
>-	int result;
>+	intmax_t result;
>+	int adjustment;
> 	int begoff;
> 	int quotes = flag & (EXP_FULL | EXP_CASE);
> 	int quoted;
>@@ -369,10 +370,9 @@
> 	 * have to rescan starting from the beginning since CTLESC
> 	 * characters have to be processed left to right.
> 	 */
>-#if INT_MAX / 1000000000 >= 10 || INT_MIN / 1000000000 <= -10
>-#error "integers with more than 10 digits are not supported"
>-#endif
>-	CHECKSTRSPACE(12 - 2, expdest);
>+/* SPACE_NEEDED is enough for all digits, plus possible "-", plus 2 (why?) */
>+#define SPACE_NEEDED ((sizeof(intmax_t) * CHAR_BIT + 2) / 3 + 1 + 2)
>+	CHECKSTRSPACE(SPACE_NEEDED - 2, expdest);
> 	USTPUTC('\0', expdest);
> 	start = stackblock();
> 	p = expdest - 1;
>@@ -394,15 +394,15 @@
> 	if (quotes)
> 		rmescapes(p+2);
> 	result = arith(p+2);
>-	fmtstr(p, 12, "%d", result);
>+	fmtstr(p, SPACE_NEEDED, "%"PRIdMAX, result);
> 
> 	while (*p++)
> 		;
> 
> 	if (quoted == 0)
> 		recordregion(begoff, p - 1 - start, 0);
>-	result = expdest - p + 1;
>-	STADJUST(-result, expdest);
>+	adjustment = expdest - p + 1;
>+	STADJUST(-adjustment, expdest);
> }
> 
> 
>Index: src/bin/sh/expand.h
>===================================================================
>--- expand.h	13 Jul 2004 15:05:59 -0000	1.16
>+++ expand.h	24 Mar 2007 12:46:15 -0000
>@@ -34,6 +34,8 @@
>  *	@(#)expand.h	8.2 (Berkeley) 5/4/95
>  */
> 
>+#include <inttypes.h>
>+
> struct strlist {
> 	struct strlist *next;
> 	char *text;
>@@ -66,7 +68,7 @@
> int wordexpcmd(int, char **);
> 
> /* From arith.y */
>-int arith(const char *);
>+intmax_t arith(const char *);
> int expcmd(int , char **);
> void arith_lex_reset(void);
> int yylex(void);
>

Looks good to me.

christos