Re: [PATCH 03/10] fdisk: API: add fdisk_label_change

[Petr Uzel <petr.uzel@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

On Tue, Jul 24, 2012 at 12:39:07PM +0200, Davidlohr Bueso wrote:
> > >> Shouldn't we use something else than hardcoded "sun"|"dos"|"sgi"|...
> > >> strings here? 
> > > 
> > > What's the problem with these strings? I think these are well
> > > recognized with clear meaning. What alternative do you propose?
> > 
> > Sorry, I should've proposed this already in my first mail:
> > 
> > -	fdisk_label_change(cxt, "sun");
> > +	fdisk_label_change(cxt, sun_label.name);
 
Ah, right. Deduplicating the strings makes sense, IMO.

> I'm planning on adding the disklabel type (numeric) to the context
> structure in a near future, so we'd then have two ways of identifying
> labels. I really do not see much difference between numbers and strings
> - this is not a performance critical program.

The concern here is not speed, but maintainability ;)

Best,

Petr

-- 
Petr Uzel
IRC: ptr_uzl @ freenode

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