Re: [linux-minidisc] [PATCH] himddump now uses getopt for option parsing

[Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

Hi Kevin

On Wed, Oct 06, 2010 at 09:40:58PM +0200, Kevin Chabowski wrote:
> Hello everyone,
>
> "https://wiki.physik.fu-berlin.de/linux-minidisc/doku.php?id=tasks"; wrote:
>> add –help option to himddump, himddiskid and himdformat  
>> (http://www.gnu.org/s/libc/manual/html_node/Getopt.html)
>
> So I replaced the option parsing code of himddump.c with a version that
> uses GNU getopt.
> Of course it also has an '--help' option ;-).
> If you like it, I can also do this for himddiskid and himdformat (should
> be no problem).

Thanks a lot for taking care of one of our tasks. I have looked
through your patch and I think I wouldn't take it into the main
repository. The reason is that you add a lot of complex code without
extending the functionality. There is already a short online help for
the himddump command so your patch doesn't add any new
functionality.

Also, you use getopt() in a way which is not intended. You use it to
set flag variables and then test afterwards which of these are set in
order to decide which command to perform. But since flags are multiple
choice and commands single choice, using getopt() with flags is not
the right choice in this case.

I would accept your patch if it reduced the amount of code or made it
more robust or if your patch added more functionality. But in the
current form, I am not going to apply it, sorry.

Of course, I am open to any comments from other people.

Again, thanks for your efforts. This has been the first patch for some
months. It's getting time to raise the efforts on the project again.


Thanks,

Adrian