Rafael Vanoni wrote:
> Aubrey Li wrote:
>> 2008/6/16 Rafael Vanoni <Rafael.Vanoni at sun.com>:
>>> Here's a diff that fixes the last three bugs listed on the powertop project
>>> page:
>>>
>>> # PowerTOP does not exit with non-zero exit code if handed an invalid option
>>> # PowerTOP works with invalid options
>>> # PowerTOP dumps usage message to stdout, not stderr
>> -    if ((bit_depth = get_bit_depth()) < 0)
>> -            return -1;
>> +    if ((bit_depth = get_bit_depth()) < 0) {
>> +            exit(EXIT_FAILURE);
>> +            return (EXIT_FAILURE);
>> +    }
>>
>> Why exit(EXIT_FAILURE) needed here?
> 
> That happens if sysinfo(2) fails to return the bit depth. Do you prefer 
> to fall back to a default value for bit depth ?
> 
>> +
>> +    if (argc > 1)
>> +            if (argv[1][0] != '-')
>> +                    usage();
>>
>> This code looks ugly, and not a fix if run "powertop --"
>> Any better idea?
> 
> getopt_long(3C) doesn't check this, so we're left with having to do it 
> manually. Do you know of a routine that we could use here ?
> 
> I've seen code in nevada that checks if the options are valid before 
> parsing them. But that's ugly code as well.

Nevermind this bit, Konstantin just pointed me to optind.
Here's a diff for that

--- a/usr/src/cmd/powertop/powertop.c   Fri May 23 10:54:13 2008 +0100
+++ b/usr/src/cmd/powertop/powertop.c   Tue Jun 17 11:33:04 2008 +0100
@@ -113,6 +113,10 @@
                        return -1;
                }
        }
+
+        if (optind < argc) {
+               usage();
+        }

        printf("Solaris PowerTOP 1.0   (C) 2007 Intel Corporation \n\n");


thanks
Rafael

Reply via email to