Re: [PATCH 1/2] block/file-posix: fix g_file_get_contents return path

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 7/5/23 10:54 AM, Matthew Rosato wrote:
> On 6/4/23 2:16 AM, Sam Li wrote:
>> The g_file_get_contents() function returns a g_boolean. If it fails, the
>> returned value will be 0 instead of -1. Solve the issue by skipping
>> assigning ret value.
>>
>> This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed
>> by an NVMe partition e.g. /dev/nvme0n1p1 on s390x.
>>
>> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> 
> Polite ping on this patch -- this issue still exists in master as of today and this patch resolves it for me.  Just want to make sure it gets into 8.1
> 

Ping -- I can still reproduce this crash on -rc1.  Any chance this patch can get picked up for the 8.1 release?

@Sam I see you sent a v2 of only patch #2 in this series ('block/file-posix: fix wps checking in raw_co_prw')..  I wonder if this one just got forgotten since it wasn't sent as part of v2.  Maybe try a resend of this patch by itself (plus the review tags added)?

Thanks,
Matt

> 
>> ---
>>  block/file-posix.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index ac1ed54811..0d9d179a35 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -1232,7 +1232,6 @@ static int hdev_get_max_hw_transfer(int fd, struct stat *st)
>>  static int get_sysfs_str_val(struct stat *st, const char *attribute,
>>                               char **val) {
>>      g_autofree char *sysfspath = NULL;
>> -    int ret;
>>      size_t len;
>>  
>>      if (!S_ISBLK(st->st_mode)) {
>> @@ -1242,8 +1241,7 @@ static int get_sysfs_str_val(struct stat *st, const char *attribute,
>>      sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s",
>>                                  major(st->st_rdev), minor(st->st_rdev),
>>                                  attribute);
>> -    ret = g_file_get_contents(sysfspath, val, &len, NULL);
>> -    if (ret == -1) {
>> +    if (!g_file_get_contents(sysfspath, val, &len, NULL)) {
>>          return -ENOENT;
>>      }
>>  
>> @@ -1253,7 +1251,7 @@ static int get_sysfs_str_val(struct stat *st, const char *attribute,
>>      if (*(p + len - 1) == '\n') {
>>          *(p + len - 1) = '\0';
>>      }
>> -    return ret;
>> +    return 0;
>>  }
>>  #endif
>>  
> 
>