[OpenAFS-devel] Re: Patch for XML switch for vos examine

[Andrew Deason]

Just one thing...

On Thu, 6 May 2010 15:01:45 +0200
Sanket Agarwal <sanket@sanketagarwal.com> wrote:

> On Wed, May 5, 2010 at 5:15 PM, Steve Simmons <scs@umich.edu> wrote:

> > /* Print value of spare3 field as integer */
> > if ( type == RAW )
> >    fprintf( STDOUT, "%spare3\t\t %d (Optional)\n", value );
> > else if ( type == XML )
> >    fprintf( STDOUT, "\t\t<spare3>%d</spare3>\n", value );
> > else if ( type == CSV )
> >    fprintf( STDOUT, "%,%d", value );

Once we get to multiple output formats, I'd have thought this to turn
into arrays of function pointers, instead of several if/else ladders (a
la the cache types in the client code). So you'd have something like

output_type->print_spare3("spare3", value);

And you'd define

static struct vos_output raw_output = {
	/* ... */
	.print_spare3 = print_raw_int_optional,
	/* ... */
};

static struct vos_output xml_output = {
	/* ... */
	.print_spare3 = print_xml_int,
	/* ... */
};

static void
print_raw_optional(const char *name, int value)
{
    fprintf(STDOUT, "%s\t\t %d (Optional)\n", name, value);
}

/* ... etc */

Or whatever. And use one of those structs depending on the -format
argument.

So, you don't have entirely separate enumeration functions, etc. What
values you print and in what order are the same for all; the only
difference is how they are output. In theory I think the problems in
e.g. SubEnumerate that Steve mentions are surmountable if you just give
each callback function enough context.

But I don't know, a lot of the logic is still scattered, so I don't know
how much better that would be. Just a thought.

-- 
Andrew Deason
adeason@sinenomine.net