Re: [hackers] [sbase][PATCH] od: Fix argument parsing for -t flag

[Michael Forney]

On 2020-02-16, Quentin Rameau  wrote:
> I think something like that is better, what do you think?

Thanks, this looks good to me. Please resend with a commit message.

> diff --git a/od.c b/od.c
> index 0b1c5c6..9ff8ff2 100644
> --- a/od.c
> +++ b/od.c
> @@ -212,7 +212,7 @@ main(int argc, char *argv[])
>  {
>   int fd;
>   struct type *t;
> - int ret = 0, len;
> + int ret = 0, len, defbytes;
>   char *s;
>
>   big_endian = (*(uint16_t *)"\0\xff" == 0xff);
> @@ -260,6 +260,7 @@ main(int argc, char *argv[])
>   case 'o':
>   case 'u':
>   case 'x':
> + defbytes = 0;
>   /* todo: allow multiple digits */
>   if (*(s+1) > '0' && *(s+1) <= '9') {
>   len = *(s+1) - '0';
> @@ -271,17 +272,17 @@ main(int argc, char *argv[])
>   case 'S':
>   len = sizeof(short);
>   break;
> + default:
> + defbytes = 1;
>   case 'I':
>   len = sizeof(int);
>   break;
>   case 'L':
>   len = sizeof(long);
>   break;
> - default:
> - len = sizeof(int);
>   }
>   }
> - addtype(*s++, len);
> + addtype(defbytes ? *s : *s++, len);

I think the increment should be separate for clarity; defbytes isn't
really controlling the parameter, just whether or not s is advanced.

addtype(*s, len);
if (!defbytes)
s++;

>   break;
>   default:
>   usage();
>
>

Re: [hackers] [sbase][PATCH] od: Fix argument parsing for -t flag

[prez]

On Sun Feb 16, 2020 at 10:36 PM, Quentin Rameau wrote:
> I think something like that is better, what do you think?
>
> 
> diff --git a/od.c b/od.c
> index 0b1c5c6..9ff8ff2 100644
> --- a/od.c
> +++ b/od.c
> @@ -212,7 +212,7 @@ main(int argc, char *argv[])
> {
> int fd;
> struct type *t;
> - int ret = 0, len;
> + int ret = 0, len, defbytes;
> char *s;
>  
> big_endian = (*(uint16_t *)"\0\xff" == 0xff);
> @@ -260,6 +260,7 @@ main(int argc, char *argv[])
> case 'o':
> case 'u':
> case 'x':
> + defbytes = 0;
> /* todo: allow multiple digits */
> if (*(s+1) > '0' && *(s+1) <= '9') {
> len = *(s+1) - '0';
> @@ -271,17 +272,17 @@ main(int argc, char *argv[])
> case 'S':
> len = sizeof(short);
> break;
> + default:
> + defbytes = 1;
> case 'I':
> len = sizeof(int);
> break;
> case 'L':
> len = sizeof(long);
> break;
> - default:
> - len = sizeof(int);
> }
> }
> - addtype(*s++, len);
> + addtype(defbytes ? *s : *s++, len);
> break;
> default:
> usage();
>
yeah, that's cleaner. thank you for correcting my attempt!

Re: [hackers] [sbase][PATCH] od: Fix argument parsing for -t flag

[Quentin Rameau]

Hi prez,

> when the -t flag is used without a number of bytes, od exits with
> usage(). 'od -tx foo' should behave like 'od -tx4 foo'.

Indeed,

> ---
>  od.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/od.c b/od.c
> index 0b1c5c6..1780710 100644
> --- a/od.c
> +++ b/od.c
> @@ -250,6 +250,7 @@ main(int argc, char *argv[])
>   break;
>   case 't':
>   s = EARGF(usage());
> + int done = 0;
>   for (; *s; s++) {
>   switch (*s) {
>   case 'a':
> @@ -277,11 +278,15 @@ main(int argc, char *argv[])
>   case 'L':
>   len = sizeof(long);
>   break;
> - default:
> + case '\0':
>   len = sizeof(int);
> + done = 1;
> + break;
> + default:
> + usage();
>   }
>   }
> - addtype(*s++, len);
> + addtype(*(done ? s : s++), len);
>   break;
>   default:
>   usage();

This is slightly wrong though, we don't want to error on default
because it's allowed to concatenate multiple types.

I think something like that is better, what do you think?

diff --git a/od.c b/od.c
index 0b1c5c6..9ff8ff2 100644
--- a/od.c
+++ b/od.c
@@ -212,7 +212,7 @@ main(int argc, char *argv[])
 {
int fd;
struct type *t;
-   int ret = 0, len;
+   int ret = 0, len, defbytes;
char *s;
 
big_endian = (*(uint16_t *)"\0\xff" == 0xff);
@@ -260,6 +260,7 @@ main(int argc, char *argv[])
case 'o':
case 'u':
case 'x':
+   defbytes = 0;
/* todo: allow multiple digits */
if (*(s+1) > '0' && *(s+1) <= '9') {
len = *(s+1) - '0';
@@ -271,17 +272,17 @@ main(int argc, char *argv[])
case 'S':
len = sizeof(short);
break;
+   default:
+   defbytes = 1;
case 'I':
len = sizeof(int);
break;
case 'L':
len = sizeof(long);
break;
-   default:
-   len = sizeof(int);
}
}
-   addtype(*s++, len);
+   addtype(defbytes ? *s : *s++, len);
break;
default:
usage();

[hackers] [sbase][PATCH] od: Fix argument parsing for -t flag

[prez]

when the -t flag is used without a number of bytes, od exits with
usage(). 'od -tx foo' should behave like 'od -tx4 foo'.

---
 od.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/od.c b/od.c
index 0b1c5c6..1780710 100644
--- a/od.c
+++ b/od.c
@@ -250,6 +250,7 @@ main(int argc, char *argv[])
break;
case 't':
s = EARGF(usage());
+   int done = 0;
for (; *s; s++) {
switch (*s) {
case 'a':
@@ -277,11 +278,15 @@ main(int argc, char *argv[])
case 'L':
len = sizeof(long);
break;
-   default:
+   case '\0':
len = sizeof(int);
+   done = 1;
+   break;
+   default:
+   usage();
}
}
-   addtype(*s++, len);
+   addtype(*(done ? s : s++), len);
break;
default:
usage();
-- 
2.20.1