Re: [PATCH 10/10] fdtdump: add a debug mode

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2068 bytes --]

On Mon, Apr 15, 2013 at 07:35:55PM -0400, Mike Frysinger wrote:
> On Monday 15 April 2013 01:12:06 David Gibson wrote:
> > On Wed, Apr 10, 2013 at 02:29:15PM -0400, Mike Frysinger wrote:
> > > -static void dump_blob(void *blob)
> > > +static const char *tagname(uint32_t tag)
> > >  {
> > > +	static const char * const names[] = {
> > > +#define TN(t) [t] #t
> > > +		TN(FDT_BEGIN_NODE),
> > > +		TN(FDT_END_NODE),
> > > +		TN(FDT_PROP),
> > > +		TN(FDT_NOP),
> > > +		TN(FDT_END),
> > > +#undef TN
> > > +	};
> > > +	if (tag < ARRAY_SIZE(names))
> > > +		if (names[tag])
> > > +			return names[tag];
> > > +	return "???";
> > 
> > Better to return NULL here, I think.  That way a caller can easily
> > check and print the numeric value instead.
> > 
> > Or you could construct a string with the number.  It would leak
> > memory, obviously, but in a short-run program like this, it doesn't
> > really matter.
> 
> if this were a general func (like in util.c or something), i'd agree with you.  
> but the only consumer also prints out the #, so it'd be redundant:
>         dprintf("%04zx: tag: 0x%08x (%s)\n",
>                 (uintptr_t)p - blob_off - 4, tag, tagname(tag));
> 
> the output looks like:
> 	// 350c: tag: 0x00000003 (FDT_PROP)
> 
> if it was an unknown value, it'd look like:
> 	// 350c: tag: 0x0000a123 (???)

Ah, good point, fair enough.

> > > +#define dprintf(fmt, args...) \
> > > +	do { if (debug) printf("// " fmt, ## args); } while (0)
> > 
> > I'd prefer a different function (well, macro) name.  I tend to use
> > "dprintf" for debug print functions in the sense of debugging for the
> > program they're in, whereas in this case it's for the debug mode which
> > is (primarily) for debugging other programs.
> 
> dumpf() ?

Not great, but nothing better springs to mind, so yeah, that'll do.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss