|
From: Vasant H. <heg...@li...> - 2016-11-21 10:36:19
|
On 11/18/2016 04:30 AM, Mauricio Faria de Oliveira wrote:
> 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?
Don't worry.. I will fix it.
>
>>> > - 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.)
I was talking about the scenario where kernel won't create sysfs node. Probably
we don't need to worry about this case for now.
>
> # 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.
This may confuse the user. I will discard above message. (will send patch for that).
> -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.
I've tweaked your patch slightly and merged it.
-Vasant
|
