Re: [PATCH v6] Introduce support for Systems Management Driver over WMI for Dell Systems

[Hans de Goede]

Hi,

On 10/26/20 4:39 PM, Limonciello, Mario wrote:
>> This was present in previous versions too, but I just noticed this are you
>> sure that using
>> .string.pointer is correct here? That seems wrong since the pointer gets
>> allocated by
>> the Linux ACPI core, so it is not under influence of the AML code?
>>
>> I think you want / need to use ".integer.value" here ?
>>
>> And maybe first do a type check, e.g.:
>>
>> 	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
>> 		ret = -EINVAL;
>> 		goto out;
>> 	}
>>
>> Adding this type check will also show if I'm right that you should use
>> .integer.value ...
>>
>> 	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj-
> 
> We'll need to double check this, but I'm pretty sure the firmware outputs
> everything as a string.

Ok, in that case it should probably be printed as a "%s" though and not interpret
the buffer-address which the kernel allocated for storing the string as an
integer.

And it would still be good to do the type-check for this, but then checking
for a type of ACPI_TYPE_STRING.

Regards,

Hans