Re: [PATCH v2 1/2] selftests/resctrl: Fix schemata write error check

[Maciej Wieczór-Retman]

Hi,
thanks for the comments!

On 2023-08-30 at 13:49:18 -0700, Reinette Chatre wrote:
>Hi Maciej,
>
>On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
>> Writing bitmasks to the schemata can fail when the bitmask doesn't
>> adhere to some constraints defined by what a particular CPU supports.
>> Some example of constraints are max length or having contiguous bits.
>> The driver should properly return errors when any rule concerning
>> bitmask format is broken.
>> 
>> Resctrl FS returns error codes from fprintf() only when fclose() is
>> called. Current error checking scheme allows invalid bitmasks to be
>> written into schemata file and the selftest doesn't notice because the
>> fclose() error code isn't checked.
>> 
>> Substitute fopen(), flose() and fprintf() with open(), close() and
>> write() to avoid error code buffering between fprintf() and fclose().
>> 
>> Add newline to the end of the schema string so it satisfies rdt
>> schemata writing requirements.
>
>I am not sure how to interpret the above because existing code already
>adds a newline to the end of the schema when the buffer is written to

Okay, true. I meant I moved it a few lines back but there is no real
change there. I'll just remove this paragraph.

>the schemata file. Also please use "resctrl schemata" since RDT is
>Intel specific and does not use schemata terminology.

Thank you, I'll change it.

>> 
>> Remove newline character from the schemat string after writing it to
>> the schemata file so it prints correctly before function return.
>
>schemat -> "schema" or "schemata"?

I'll stick with schema as that's the variable name, thanks for finding
this typo.

>> Pass the string generated with strerror() to the "reason" buffer so
>> the error message is more verbose. Extend "reason" buffer so it can hold
>> longer messages.
>> 
>> Changelog v2:
>> - Rewrite patch message.
>> - Double "reason" buffer size to fit longer error explanation.
>> - Redo file interactions with syscalls instead of stdio functions.
>> 
>
>Please place the above "Changelog v2" snippet below the "---" lines below.
>This is text that should not end up in the kernel log.

Yes, I realized I made this mistake a few hours after sending the patch.
I'll correct it and make sure to double check it before sending next time.

>> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@xxxxxxxxx>
>> ---
>
>(list of changes should go here)
>
>Reinette
>

-- 
Kind regards
Maciej Wieczór-Retman