Bug#321109: [PATCH] grub-installer password confirmation

[James Westby <jw+debian@jameswestby.net>]

On Thu, 2008-04-17 at 16:58 +0200, Frans Pop wrote:
> After a quick glance a couple of things look wrong.
> 

Hi,

Thanks for the review.

> You read password-crypted to see if it is preseeded. This value presumably 
> already _is_ encrypted, but you still pass it through 'password --md5'.
> 

The encryption call is in the else branch, and so it should not
happen when the value was preseeded. There is "password --md5" echoed
to the tempfile, but this just indicates that the password is encrypted.

> Also, why is the output of the password command _appended_ to the temp file? 
> That would seem to break idempotency.

That's just because it follows the code that was there before, I'd
be happy to test it if you think that it would be necessary.

The temp file is removed after the sed command that puts it in to the
menu.lst. Is there an issue that the sed command would end up being run
twice, giving you two password lines in the final menu.lst?

> 
> In general, how well has this patch been tested, both for interactive use 
> and preseeding? Has entering the password been tested after a reboot in 
> both cases?

I haven't tested it with preseeding. Entering the password after reboot
does work for the non-preseeded case.

Thanks,

James