jot: small cleanup for conversion switch

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

jot: small cleanup for conversion switch

Theo Buehler-3
This aligns all cases vertically which makes them easier to find.

Normalize all cases: if the long form is illegal or unsupported,
'goto fmt_broken;', then set the flags for the casts in putdata() on a
single line.

This is all straightforward, but I think the resulting code is easier to
follow. Of note is the case of '%c' which used to check whether intdata
is set. This is impossible, so I dropped that bit.

Regression tests still happy.

Index: jot.c
===================================================================
RCS file: /var/cvs/src/usr.bin/jot/jot.c,v
retrieving revision 1.42
diff -u -p -r1.42 jot.c
--- jot.c 11 Jan 2018 14:53:42 -0000 1.42
+++ jot.c 11 Jan 2018 20:08:16 -0000
@@ -398,38 +398,41 @@ getformat(void)
  }
  }
  switch (*p) {
- case 'o': case 'u': case 'x': case 'X':
- intdata = nosign = true;
- break;
- case 'd': case 'i':
+ case 'd':
+ case 'i':
  intdata = true;
  break;
+ case 'o':
+ case 'u':
+ case 'x':
+ case 'X':
+ intdata = nosign = true;
+ break;
  case 'D':
- /* %lD is undefined */
- if (!longdata) {
- longdata = true; /* %D behaves as %ld */
- intdata = true;
- break;
- }
- goto fmt_broken;
- case 'O': case 'U':
- /* %lO and %lU are undefined */
- if (!longdata) {
- longdata = true; /* %O, %U behave as %lo, %lu */
- intdata = nosign = true;
- break;
- }
- goto fmt_broken;
+ if (longdata)
+ goto fmt_broken;
+ longdata = intdata = true; /* same as %ld */
+ break;
+ case 'O':
+ case 'U':
+ if (longdata)
+ goto fmt_broken;
+ longdata = intdata = nosign = true; /* same as %l[ou] */
+ break;
  case 'c':
- if (!(intdata | longdata)) {
- chardata = true;
- break;
- }
- goto fmt_broken;
- case 'f': case 'e': case 'g': case 'E': case 'G':
- if (!longdata)
- break;
- /* FALLTHROUGH */
+ if (longdata)
+ goto fmt_broken;
+ chardata = true;
+ break;
+ case 'e':
+ case 'E':
+ case 'f':
+ case 'g':
+ case 'G':
+ if (longdata)
+ goto fmt_broken;
+ /* No cast needed for printing in putdata() */
+ break;
  default:
 fmt_broken:
  errx(1, "illegal or unsupported format '%.*s'",

Reply | Threaded
Open this post in threaded view
|

Re: jot: small cleanup for conversion switch

Alexander Hall
Didn't test, but reads ok to me, with minor nit below.

On January 11, 2018 9:25:10 PM GMT+01:00, Theo Buehler <[hidden email]> wrote:

>This aligns all cases vertically which makes them easier to find.
>
>Normalize all cases: if the long form is illegal or unsupported,
>'goto fmt_broken;', then set the flags for the casts in putdata() on a
>single line.
>
>This is all straightforward, but I think the resulting code is easier
>to
>follow. Of note is the case of '%c' which used to check whether intdata
>is set. This is impossible, so I dropped that bit.
>
>Regression tests still happy.
>
>Index: jot.c
>===================================================================
>RCS file: /var/cvs/src/usr.bin/jot/jot.c,v
>retrieving revision 1.42
>diff -u -p -r1.42 jot.c
>--- jot.c 11 Jan 2018 14:53:42 -0000 1.42
>+++ jot.c 11 Jan 2018 20:08:16 -0000
>@@ -398,38 +398,41 @@ getformat(void)
> }
> }
> switch (*p) {
>- case 'o': case 'u': case 'x': case 'X':
>- intdata = nosign = true;
>- break;
>- case 'd': case 'i':
>+ case 'd':
>+ case 'i':
> intdata = true;
> break;
>+ case 'o':
>+ case 'u':
>+ case 'x':
>+ case 'X':

Convention is X before x in usage() and friends, so I guess that'd make sense here (and below) too.

/Alexander

>+ intdata = nosign = true;
>+ break;
> case 'D':
>- /* %lD is undefined */
>- if (!longdata) {
>- longdata = true; /* %D behaves as %ld */
>- intdata = true;
>- break;
>- }
>- goto fmt_broken;
>- case 'O': case 'U':
>- /* %lO and %lU are undefined */
>- if (!longdata) {
>- longdata = true; /* %O, %U behave as %lo, %lu */
>- intdata = nosign = true;
>- break;
>- }
>- goto fmt_broken;
>+ if (longdata)
>+ goto fmt_broken;
>+ longdata = intdata = true; /* same as %ld */
>+ break;
>+ case 'O':
>+ case 'U':
>+ if (longdata)
>+ goto fmt_broken;
>+ longdata = intdata = nosign = true; /* same as %l[ou] */
>+ break;
> case 'c':
>- if (!(intdata | longdata)) {
>- chardata = true;
>- break;
>- }
>- goto fmt_broken;
>- case 'f': case 'e': case 'g': case 'E': case 'G':
>- if (!longdata)
>- break;
>- /* FALLTHROUGH */
>+ if (longdata)
>+ goto fmt_broken;
>+ chardata = true;
>+ break;
>+ case 'e':
>+ case 'E':
>+ case 'f':
>+ case 'g':
>+ case 'G':
>+ if (longdata)
>+ goto fmt_broken;
>+ /* No cast needed for printing in putdata() */
>+ break;
> default:
> fmt_broken:
> errx(1, "illegal or unsupported format '%.*s'",

Reply | Threaded
Open this post in threaded view
|

Re: jot: small cleanup for conversion switch

Theo Buehler-3
> >+ case 'd':
> >+ case 'i':
> > intdata = true;
> > break;
> >+ case 'o':
> >+ case 'u':
> >+ case 'x':
> >+ case 'X':
>
> Convention is X before x in usage() and friends, so I guess that'd
> make sense here (and below) too.

Thanks. I deliberately used the same order as in printf(3) and the C11
standard, so I left that as it was. If this makes your eyes twitch too
hard, feel free to change it. :)

Reply | Threaded
Open this post in threaded view
|

Re: jot: small cleanup for conversion switch

Alexander Hall


On January 12, 2018 7:26:58 AM GMT+01:00, Theo Buehler <[hidden email]> wrote:

>> >+ case 'd':
>> >+ case 'i':
>> > intdata = true;
>> > break;
>> >+ case 'o':
>> >+ case 'u':
>> >+ case 'x':
>> >+ case 'X':
>>
>> Convention is X before x in usage() and friends, so I guess that'd
>> make sense here (and below) too.
>
>Thanks. I deliberately used the same order as in printf(3) and the C11
>standard, so I left that as it was. If this makes your eyes twitch too
>hard, feel free to change it. :)

Sounds reasonable, and like a fair deal. :-)