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
