On Tuesday 27 May 2008, Colin Watson wrote:
> > > create_new_partition () {
> > > - local num id size type fs path name mp mplist mpcurrent numparts
> > > device open_dialog NEW_PARTITION $type ext2 $freeid $place $size
> > > + local num id size type fs path name mp mplist mpcurrent numparts
> > > device id=''
> > > read_line num id size type fs path name
> > > close_dialog
>
> > It looks wrong to me as now some vars are being defined as local after
> > they are first used.
>
> No, that isn't the case. ask_for_type sets the 'type' variable which is
> used at the top of create_new_partition. create_new_partition then
> reuses type for something entirely different (and localises it in the
> process).
Ah, so the vars in the 'device open_dialog' line are actually global vars
set outside the function? That is very ugly coding for partman standards
and should definitely be fixed.
> > Should the local line maybe be split into two lines instead?
>
> Probably wouldn't hurt to rename the type variable in
> create_new_partition, but there's nothing actually incorrect about my
> change as far as I can see.
It's not just 'type', it is 'size' as well.
My preference would be to remove type and size from the first 'local'
declaration and have a second 'local' declaration after the open_dialog
call with just those two vars. Probably add a comment as well.
Only problem with you change is that someone might be inclined to move the
line back up...
Attachment:
signature.asc
Description: This is a digitally signed message part.