The status local variable is assigned but never read:
drivers/platform/surface/surface3-wmi.c:60:14: warning:
variable ‘status’ set but not used [-Wunused-but-set-variable]
acpi_status status;
^~~~~~
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Zou Wei <[email protected]>
---
drivers/platform/surface/surface3-wmi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/platform/surface/surface3-wmi.c b/drivers/platform/surface/surface3-wmi.c
index 130b6f5..ae1416c 100644
--- a/drivers/platform/surface/surface3-wmi.c
+++ b/drivers/platform/surface/surface3-wmi.c
@@ -57,12 +57,10 @@ static DEFINE_MUTEX(s3_wmi_lock);
static int s3_wmi_query_block(const char *guid, int instance, int *ret)
{
struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
- acpi_status status;
union acpi_object *obj;
int error = 0;
mutex_lock(&s3_wmi_lock);
- status = wmi_query_block(guid, instance, &output);
obj = output.pointer;
--
2.6.2
On Thu, Oct 29, 2020 at 1:36 PM Zou Wei <[email protected]> wrote:
>
> The status local variable is assigned but never read:
...
> mutex_lock(&s3_wmi_lock);
> - status = wmi_query_block(guid, instance, &output);
What are you doing?!
--
With Best Regards,
Andy Shevchenko
On 10/29/20 12:47 PM, Zou Wei wrote:
> The status local variable is assigned but never read:
[...]
> static int s3_wmi_query_block(const char *guid, int instance, int *ret)
> {
> struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> - acpi_status status;
> union acpi_object *obj;
> int error = 0;
>
> mutex_lock(&s3_wmi_lock);
> - status = wmi_query_block(guid, instance, &output);
I assume you want to remove the "status =" and not the whole line...
Regardless, I personally would prefer explicit error handling (as is
done in other contexts with wmi_query_block(), see e.g.
platform/x86/intel-wmi-sbl-fw-update.c. Although, yes, the "if (!obj |
..." should cover the error case as far as I can tell.
Also there seems to be the same issue in platform/x86/msi-wmi.c.
On a related note:
Does anyone know if the mutex here is really required? If there are any
problems, I believe it should rather go to s3_wmi_send_lid_state() and
also wrap the input_report_switch() / input_sync(). Otherwise, if there
were any race-type situations, not covering that could lead to wrong
updates:
Thread 1 Thread 2
s3_query_lid() returns old
s3_query_lid() returns new
input_report_switch()
input_sync()
input_report_switch()
input_sync()
I don't really expect those situations here as s3_wmi_send_lid_state()
is only used in probe(), resume(), and notify() (please correct me if
I'm wrong), so the mutex does seem weird to me here.
Otherwise if it's needed, a comment explaining why wouldn't hurt.
Regards,
Max