Hi Vasant,
On 11/17/2016 12:42 PM, Vasant Hegde wrote:
>> > - opal_get_indicator_mode()
>> > We need to add check in this function.
>>
>> Can you clarify this request?
> Now that we have special case, we should fix this function.
Sorry; even w/ your explanation I didn't get why/what exactly
needs to be fixed in this function. Can you say it for dummies?
>> > - opal_get_indicator_list()
[snip]
> opal_get_indicator_list -> opal_get_indices -> sysfs led class dir
> not exists -> return -1 ..
[snip]
>> - the other call is for opal_get_indices(),
>> which does not fail because scandir() should have returned 0
>> (no entries), not -1 in case of error.
>
> On error scandir returns -1 right?
Yes, but is doesn't error in this case. The /sys/class/leds dir is
present even without LED modules loaded (led-class.o is built-in.)
# ls /sys/class/leds
#
# lsmod | grep -i led
#
# ./lpd/usysident
Some service indicators are not supported on this system.
Make sure 'leds_powernv' kernel module is loaded.
-B0-T0-L0 [on]
-B0-T0-L0 [off]
And on empty dirs, its rc is 2 (for . and ..)
$ cat scandir.c
#include <dirent.h>
#include <stdlib.h>
int main() {
struct dirent **dirlist;
return scandir("emptydir", &dirlist, NULL, alphasort);
}
$ gcc -o scandir scandir.c
$ mkdir emptydir
$ ./scandir; echo $?
2
$ rmdir emptydir
$ ./scandir; echo $?
255
Well, but with the patch for the section below, I guess this topic
is no longer necessary :- ) as a patch makes opal_get_indicator_list()
not to return early if opal_get_indices() fails.
> So now we have number of sources to get LEDs (sysfs, ses_leds, marvel
> leds).
> If we fail to get LEDs from one source, lets continue with other sources
> and build list instead of returning .
Okay, submitting a patch for this. It checks whether any of the function
calls initialized the list pointer to non-NULL (ie, allocated an entry
for an indicator).
Would you please check if the assumption to remove the fault mode check
is correct? I'm afraid I don't understand enough about this to be sure.
Thanks!
--
Mauricio Faria de Oliveira
IBM Linux Technology Center
|