linux-kernel - Re: [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dadb7068-e4ed-b91d-ce31-aca2d6afd9f3@users.sourceforge.net>
Date:   Tue, 31 Oct 2017 15:24:05 +0100
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        platform-driver-x86@...r.kernel.org
Cc:     Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Mattia Dongili <malattia@...ux.it>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] Sony-laptop: Use common error handling code in
 sony_nc_setup_rfkill()

> Apparently this patch done without reading the actual code.

I performed another simple software refactoring.

> NAK.
> 
> -1 is EPERM which sounds wrong here. If you would like to fix it,
> propagate a real error codes from sony_call_snc_handle().

I do not know at the moment why an error code which you find
questionable was suggested by Marco Chiappero on 2012-05-19
and committed by Matthew Garrett on 2012-05-31 (according the
commit d6f15ed876b83a1a0eba1d0473eef58acc95444a: use soft rfkill
status stored in hw).

>>         if (!rfk)
>>                 return -ENOMEM;
> 
> Okay error code.

Can any other identifier make more sense there?

>> -       }
>> +       if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0)
>> +               goto destroy_rfk;
>> +
>>         hwblock = !(result & 0x1);
>>
>>         if (sony_call_snc_handle(sony_rfkill_handle,
>> -                               sony_rfkill_address[nc_type],
>> -                               &result) < 0) {
>> -               rfkill_destroy(rfk);
>> -               return -1;
> 
> Not okay and it might be different from previous case.

The shown function call was repeated with an other parameter.
Which error reporting would you find more appropriate then?

> P.S. Don't bother us with patches on which you didn't do your home work.

Do we occasionally need to distinguish better between an ordinary
refactoring and the adjustments for additional software improvements?

Regards,
Markus