lkml.org 
[lkml]   [2021]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member
From
Date
Hi Martin,


On 3/25/21 22:34, Martin K. Petersen wrote:
>
> Gustavo,
>
>> Precisely this sort of confusion is one of the things we want to avoid
>> by using flexible-array members instead of one-element arrays.
>
> Ah, you're right!
>
> Now that I look at it again I also don't think that was the issue that
> originally caused concern.
>
> @@ -4020,7 +4020,8 @@ static int aac_convert_sgraw2(struct aac_raw_io2 *rio2, int pages, int nseg, int
> }
> }
> sge[pos] = rio2->sge[nseg-1];
> - memcpy(&rio2->sge[1], &sge[1], (nseg_new-1)*sizeof(struct sge_ieee1212));
> + memcpy(&rio2->sge[1], &sge[1],
> + flex_array_size(rio2, sge, nseg_new - 1));
>
> kfree(sge);
> rio2->sgeCnt = cpu_to_le32(nseg_new);
>
> I find it counter-intuitive to use the type of the destination array to
> size the amount of source data to copy. "Are source and destination same

The destination and source arrays are of the same type. :)

drivers/scsi/aacraid/aachba.c:
3999 struct sge_ieee1212 *sge;

> type? Does flex_array_size() do the right thing given the ->sge[1]
> destination offset?". It wasn't immediately obvious. To me, "copy this
> many scatterlist entries" in the original is much more readable.

Yeah; it does the right thing because flex_array_size() doesn't know about
offsets. It just calculates the amount of bytes to be copied based on the
type of the object passed as second argument and a "count" passed as third
argument. So, in this case, the "count" is "nseg_new - 1", which in some
way is already taking care of that sge[1] offset.

--
Gustavo

\
 
 \ /
  Last update: 2021-03-26 05:31    [W:0.063 / U:0.712 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site