On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote:
>Hi Maciej,
>When I build the tests with this applied I encounter the following:
>
>resctrlfs.c: In function ‘write_schemata’:
>resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration]
> 475 | fd = open(controlgroup, O_WRONLY);
> | ^~~~
> | popen
>resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function)
> 475 | fd = open(controlgroup, O_WRONLY);
> | ^~~~~~~~
>resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in
Hmm, that's odd. How do you build the tests?
I use "make -C tools/testing/selftests/resctrl" while in the root kernel
source directory. I tried to get the same error you experienced by
compiling some dummy test program with "open" and "O_WRONLY". From the
experiment I found that the "resctrl.h" header provides the declarations
that are causing your errors.
>> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
>> index bd36ee206602..b0b14a5bcbf5 100644
>> --- a/tools/testing/selftests/resctrl/resctrlfs.c
>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
>> @@ -488,9 +488,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
>> */
>> int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
>> {
>> - char controlgroup[1024], schema[1024], reason[64];
>> - int resource_id, ret = 0;
>> - FILE *fp;
>> + char controlgroup[1024], schema[1024], reason[128];
>> + int resource_id, fd, schema_len = -1, ret = 0;
>>
>> if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
>> strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) &&
>> @@ -518,27 +517,30 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
>>
>> if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
>> !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
>> - sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata);
>> + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
>> + "L3:", resource_id, '=', schemata);
>> if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||
>> !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
>> - sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata);
>> + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
>> + "MB:", resource_id, '=', schemata);
>>
>> - fp = fopen(controlgroup, "w");
>> - if (!fp) {
>> + fd = open(controlgroup, O_WRONLY);
>> + if (!fd) {
>> sprintf(reason, "Failed to open control group");
>
>It makes code easier to understand and maintain if it is kept
>consistent. It is thus unexpected for open() error handling to
>be untouched while write() error handling is modified. I think
>the addition of errno in error handling of write() is helpful.
>Could you do the same for open()?
Okay, I'll add that, thanks.
--
Kind regards
Maciej Wieczór-Retman
