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.
>> I also would like to remove the following piece of code from powertop.c
>>
>> /*
>> * Quiet down the effects of any IO to xterms
>> */
>> if (!key && ticktime >= 4.8) {
>> FD_ZERO(&rfds);
>> FD_SET(0, &rfds);
>> tv.tv_sec = 3;
>> tv.tv_usec = 0;
>> key = select(1, &rfds, NULL, NULL, &tv);
>> }
>>
>> As the expected functionality is broken and is causing these two bugs:
>>
>> # Time interval is not constant
>> # Multiple PowerTop instances experience time interval fluctuation
>>
>> An alternative would be to calculate the '4.8' during execution, but I don't
>> know what to base it on.
>>
>> What do you think ?
>
> Why time interval has to be constant?
Hmm.. is the expected behavior to have the interval fluctuating ?
I thought that if the user specified an interval, then he wanted it to
be constant.
thanks
Rafael
