archive mirror
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <>
To: "Gustavo A. R. Silva" <>
Cc: "Martin K. Petersen" <>,
	"Gustavo A. R. Silva" <>,
	Adaptec OEM Raid Solutions <>,
	"James E.J. Bottomley" <>,,,
Subject: Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member
Date: Thu, 25 Mar 2021 23:34:27 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Gustavo A. R. Silva's message of "Wed, 24 Mar 2021 19:46:35 -0500")


> 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));
 	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
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.

That said, this whole function makes my head hurt!

Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2021-03-26  3:35 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-04 20:38 Gustavo A. R. Silva
2021-03-25  1:18 ` Martin K. Petersen
2021-03-25  0:46   ` Gustavo A. R. Silva
2021-03-26  3:34     ` Martin K. Petersen [this message]
2021-03-26  3:07       ` Gustavo A. R. Silva
2021-04-07 19:22 ` Kees Cook
2021-04-13  4:52   ` Martin K. Petersen
2021-04-13  5:45     ` Gustavo A. R. Silva
2021-04-13 14:04       ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).