Re: [hackers] sbase: od: added -t option

From: FRIGN <dev_AT_frign.de>
Date: Tue, 29 Sep 2015 10:57:27 +0200

On Mon, 28 Sep 2015 23:06:37 -0400
Greg Reagle <greg.reagle_AT_umbc.edu> wrote:

Hey Greg,

> See attached patch.

looking good, just some points as usual:

1) escaped_char(unsigned char c)
   named_char(unsigned char c)

   Maybe it's better to use a small lookup
   table here. I know the smart compilers
   convert long switches to tables anyway,
   but it would clear up the code a bit.
   Limiting early returns inside a function
   is always desired.
   While talking about it, in general we don't
   put parentheses around return-statements. ;)

2) The use of radix is a bit puzzling, maybe I'm
   just missing something. Why is radix passed to
   print_address when we already have a global var?
   Or does it change based on context?
   Why not: "static char address_radix = 'c'"?

Thanks for your work! :)

Cheers

FRIGN

-- 
FRIGN <dev_AT_frign.de>
Received on Tue Sep 29 2015 - 10:57:27 CEST

This archive was generated by hypermail 2.3.0 : Tue Sep 29 2015 - 11:00:12 CEST