[PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Gustavo A. R. Silva]

There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].

Refactor the code according to the use of a flexible-array member in
struct aac_raw_io2 instead of one-element array, and use the
struct_size() and flex_array_size() helpers.

Also, this helps with the ongoing efforts to enable -Warray-bounds by
fixing the following warnings:

drivers/scsi/aacraid/aachba.c: In function ‘aac_build_sgraw2’:
drivers/scsi/aacraid/aachba.c:3970:18: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 3970 |     if (rio2->sge[j].length % (i*PAGE_SIZE)) {
      |         ~~~~~~~~~^~~
drivers/scsi/aacraid/aachba.c:3974:27: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 3974 |     nseg_new += (rio2->sge[j].length / (i*PAGE_SIZE));
      |                  ~~~~~~~~~^~~
drivers/scsi/aacraid/aachba.c:4011:28: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 4011 |   for (j = 0; j < rio2->sge[i].length / (pages * PAGE_SIZE); ++j) {
      |                   ~~~~~~~~~^~~
drivers/scsi/aacraid/aachba.c:4012:24: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 4012 |    addr_low = rio2->sge[i].addrLow + j * pages * PAGE_SIZE;
      |               ~~~~~~~~~^~~
drivers/scsi/aacraid/aachba.c:4014:33: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 4014 |    sge[pos].addrHigh = rio2->sge[i].addrHigh;
      |                        ~~~~~~~~~^~~
drivers/scsi/aacraid/aachba.c:4015:28: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
 4015 |    if (addr_low < rio2->sge[i].addrLow)
      |                   ~~~~~~~~~^~~

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays

Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Build-tested-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/60414244.ur4%2FkI+fBF1ohKZs%25lkp@intel.com/
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/scsi/aacraid/aachba.c  | 13 +++++++------
 drivers/scsi/aacraid/aacraid.h |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 4ca5e13a26a6..0f5617e40b94 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1235,8 +1235,8 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
 		if (ret < 0)
 			return ret;
 		command = ContainerRawIo2;
-		fibsize = sizeof(struct aac_raw_io2) +
-			((le32_to_cpu(readcmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
+		fibsize = struct_size(readcmd2, sge,
+				     le32_to_cpu(readcmd2->sgeCnt));
 	} else {
 		struct aac_raw_io *readcmd;
 		readcmd = (struct aac_raw_io *) fib_data(fib);
@@ -1366,8 +1366,8 @@ static int aac_write_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
 		if (ret < 0)
 			return ret;
 		command = ContainerRawIo2;
-		fibsize = sizeof(struct aac_raw_io2) +
-			((le32_to_cpu(writecmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
+		fibsize = struct_size(writecmd2, sge,
+				      le32_to_cpu(writecmd2->sgeCnt));
 	} else {
 		struct aac_raw_io *writecmd;
 		writecmd = (struct aac_raw_io *) fib_data(fib);
@@ -4003,7 +4003,7 @@ static int aac_convert_sgraw2(struct aac_raw_io2 *rio2, int pages, int nseg, int
 	if (aac_convert_sgl == 0)
 		return 0;
 
-	sge = kmalloc_array(nseg_new, sizeof(struct sge_ieee1212), GFP_ATOMIC);
+	sge = kmalloc_array(nseg_new, sizeof(*sge), GFP_ATOMIC);
 	if (sge == NULL)
 		return -ENOMEM;
 
@@ -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);
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index e3e4ecbea726..3733df77bc65 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1929,7 +1929,7 @@ struct aac_raw_io2 {
 	u8		bpComplete;	/* reserved for F/W use */
 	u8		sgeFirstIndex;	/* reserved for F/W use */
 	u8		unused[4];
-	struct sge_ieee1212	sge[1];
+	struct sge_ieee1212	sge[];
 };
 
 #define CT_FLUSH_CACHE 129
-- 
2.27.0

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Martin K. Petersen]

Hi Gustavo!

Your changes and the original code do not appear to be functionally
equivalent.

> @@ -1235,8 +1235,8 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
>  		if (ret < 0)
>  			return ret;
>  		command = ContainerRawIo2;
> -		fibsize = sizeof(struct aac_raw_io2) +
> -			((le32_to_cpu(readcmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
> +		fibsize = struct_size(readcmd2, sge,
> +				     le32_to_cpu(readcmd2->sgeCnt));

The old code allocated sgeCnt-1 elements (whether that was a mistake or
not I do not know) whereas the new code would send a larger fib to the
ASIC. I don't have any aacraid adapters and I am hesitant to merging
changes that have not been validated on real hardware.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Gustavo A. R. Silva]

Hi Martin,

On 3/24/21 20:18, Martin K. Petersen wrote:
> 
> Hi Gustavo!
> 
> Your changes and the original code do not appear to be functionally
> equivalent.
> 
>> @@ -1235,8 +1235,8 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
>>  		if (ret < 0)
>>  			return ret;
>>  		command = ContainerRawIo2;
>> -		fibsize = sizeof(struct aac_raw_io2) +
>> -			((le32_to_cpu(readcmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
>> +		fibsize = struct_size(readcmd2, sge,
>> +				     le32_to_cpu(readcmd2->sgeCnt));
> 
> The old code allocated sgeCnt-1 elements (whether that was a mistake or
> not I do not know) whereas the new code would send a larger fib to the
> ASIC. I don't have any aacraid adapters and I am hesitant to merging
> changes that have not been validated on real hardware.

Precisely this sort of confusion is one of the things we want to avoid
by using flexible-array members instead of one-element arrays.

fibsize is actually the same for both the old and the new code. The
difference is that in the original code, the one-element array _sge_
at the bottom of struct aac_raw_io2, contributes to the size of the
structure, as it occupies at least as much space as a single object
of its type. On the other hand, flexible-array members don't contribute
to the size of the enclosing structure. See below...

Old code:

$ pahole -C aac_raw_io2 drivers/scsi/aacraid/aachba.o
struct aac_raw_io2 {
	__le32                     blockLow;             /*     0     4 */
	__le32                     blockHigh;            /*     4     4 */
	__le32                     byteCount;            /*     8     4 */
	__le16                     cid;                  /*    12     2 */
	__le16                     flags;                /*    14     2 */
	__le32                     sgeFirstSize;         /*    16     4 */
	__le32                     sgeNominalSize;       /*    20     4 */
	u8                         sgeCnt;               /*    24     1 */
	u8                         bpTotal;              /*    25     1 */
	u8                         bpComplete;           /*    26     1 */
	u8                         sgeFirstIndex;        /*    27     1 */
	u8                         unused[4];            /*    28     4 */
	struct sge_ieee1212        sge[1];               /*    32    16 */

	/* size: 48, cachelines: 1, members: 13 */
	/* last cacheline: 48 bytes */
};

New code:

$ pahole -C aac_raw_io2 drivers/scsi/aacraid/aachba.o
struct aac_raw_io2 {
	__le32                     blockLow;             /*     0     4 */
	__le32                     blockHigh;            /*     4     4 */
	__le32                     byteCount;            /*     8     4 */
	__le16                     cid;                  /*    12     2 */
	__le16                     flags;                /*    14     2 */
	__le32                     sgeFirstSize;         /*    16     4 */
	__le32                     sgeNominalSize;       /*    20     4 */
	u8                         sgeCnt;               /*    24     1 */
	u8                         bpTotal;              /*    25     1 */
	u8                         bpComplete;           /*    26     1 */
	u8                         sgeFirstIndex;        /*    27     1 */
	u8                         unused[4];            /*    28     4 */
	struct sge_ieee1212        sge[];                /*    32     0 */

	/* size: 32, cachelines: 1, members: 13 */
	/* last cacheline: 32 bytes */
};

So, the old code allocates sgeCnt-1 elements because sizeof(struct aac_raw_io2) is
already counting one element of the _sge_ array.

Please, let me know if this is clear now.

Thanks!
--
Gustavo

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Martin K. Petersen]

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
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

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Gustavo A. R. Silva]

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

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Kees Cook]

On Thu, Mar 04, 2021 at 02:38:22PM -0600, Gustavo A. R. Silva wrote:
> There is a regular need in the kernel to provide a way to declare having
> a dynamically sized set of trailing elements in a structure. Kernel code
> should always use “flexible array members”[1] for these cases. The older
> style of one-element or zero-length arrays should no longer be used[2].
> 
> Refactor the code according to the use of a flexible-array member in
> struct aac_raw_io2 instead of one-element array, and use the
> struct_size() and flex_array_size() helpers.
> 
> Also, this helps with the ongoing efforts to enable -Warray-bounds by
> fixing the following warnings:
> 
> drivers/scsi/aacraid/aachba.c: In function ‘aac_build_sgraw2’:
> drivers/scsi/aacraid/aachba.c:3970:18: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  3970 |     if (rio2->sge[j].length % (i*PAGE_SIZE)) {
>       |         ~~~~~~~~~^~~
> drivers/scsi/aacraid/aachba.c:3974:27: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  3974 |     nseg_new += (rio2->sge[j].length / (i*PAGE_SIZE));
>       |                  ~~~~~~~~~^~~
> drivers/scsi/aacraid/aachba.c:4011:28: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  4011 |   for (j = 0; j < rio2->sge[i].length / (pages * PAGE_SIZE); ++j) {
>       |                   ~~~~~~~~~^~~
> drivers/scsi/aacraid/aachba.c:4012:24: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  4012 |    addr_low = rio2->sge[i].addrLow + j * pages * PAGE_SIZE;
>       |               ~~~~~~~~~^~~
> drivers/scsi/aacraid/aachba.c:4014:33: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  4014 |    sge[pos].addrHigh = rio2->sge[i].addrHigh;
>       |                        ~~~~~~~~~^~~
> drivers/scsi/aacraid/aachba.c:4015:28: warning: array subscript 1 is above array bounds of ‘struct sge_ieee1212[1]’ [-Warray-bounds]
>  4015 |    if (addr_low < rio2->sge[i].addrLow)
>       |                   ~~~~~~~~~^~~
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/109
> Build-tested-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/60414244.ur4%2FkI+fBF1ohKZs%25lkp@intel.com/
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/scsi/aacraid/aachba.c  | 13 +++++++------
>  drivers/scsi/aacraid/aacraid.h |  2 +-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 4ca5e13a26a6..0f5617e40b94 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -1235,8 +1235,8 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
>  		if (ret < 0)
>  			return ret;
>  		command = ContainerRawIo2;
> -		fibsize = sizeof(struct aac_raw_io2) +
> -			((le32_to_cpu(readcmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
> +		fibsize = struct_size(readcmd2, sge,
> +				     le32_to_cpu(readcmd2->sgeCnt));

readcmd2 is struct aac_raw_io2, and sge is the struct sge_ieee1212
array, so this looks correct to me with the change to struct
aac_raw_io2..

>  	} else {
>  		struct aac_raw_io *readcmd;
>  		readcmd = (struct aac_raw_io *) fib_data(fib);
> @@ -1366,8 +1366,8 @@ static int aac_write_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
>  		if (ret < 0)
>  			return ret;
>  		command = ContainerRawIo2;
> -		fibsize = sizeof(struct aac_raw_io2) +
> -			((le32_to_cpu(writecmd2->sgeCnt)-1) * sizeof(struct sge_ieee1212));
> +		fibsize = struct_size(writecmd2, sge,
> +				      le32_to_cpu(writecmd2->sgeCnt));

writecmd2 is struct aac_raw_io2, and sge is the struct sge_ieee1212
array, so this looks correct to me with the change to struct
aac_raw_io2.

>  	} else {
>  		struct aac_raw_io *writecmd;
>  		writecmd = (struct aac_raw_io *) fib_data(fib);
> @@ -4003,7 +4003,7 @@ static int aac_convert_sgraw2(struct aac_raw_io2 *rio2, int pages, int nseg, int
>  	if (aac_convert_sgl == 0)
>  		return 0;
>  
> -	sge = kmalloc_array(nseg_new, sizeof(struct sge_ieee1212), GFP_ATOMIC);
> +	sge = kmalloc_array(nseg_new, sizeof(*sge), GFP_ATOMIC);

Technically, this is unrelated (struct sge_ieee1212 has not changed),
but sge is a struct sge_ieee1212 pointer, so this is good robustness
change, IMO.

>  	if (sge == NULL)
>  		return -ENOMEM;
>  
> @@ -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));

This was hard to validate, but looks correct to me. The flex array
helper here is the same as the prior code (but now tied to the
variables, which is more robust IMO). The use of seg[1] here appears to
be just how this code works -- the loop above is rewriting the 1 through
nseg_new - 1 array entries, and then this copies back the results.

>  
>  	kfree(sge);
>  	rio2->sgeCnt = cpu_to_le32(nseg_new);
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index e3e4ecbea726..3733df77bc65 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -1929,7 +1929,7 @@ struct aac_raw_io2 {
>  	u8		bpComplete;	/* reserved for F/W use */
>  	u8		sgeFirstIndex;	/* reserved for F/W use */
>  	u8		unused[4];
> -	struct sge_ieee1212	sge[1];
> +	struct sge_ieee1212	sge[];
>  };
>  
>  #define CT_FLUSH_CACHE 129
> -- 
> 2.27.0
> 

Thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Martin K. Petersen]

Hi Kees/Gustavo!

>> @@ -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));
>
> This was hard to validate, 

... which is why I didn't apply this patch. I don't like changes which
make the reader have to jump through hoops to figure out what the code
actually does. I find the original much easier to understand.

Silencing analyzer warnings shouldn't be done at the expense of human
readers. If it is imperative to switch to flex_array_size() to quiesce
checker warnings, please add a comment in the code explaining that the
size evaluates to nseg_new-1 sge_ieee1212 structs.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[Gustavo A. R. Silva]

Hi Martin,

On 4/12/21 23:52, Martin K. Petersen wrote:

> Silencing analyzer warnings shouldn't be done at the expense of human
> readers. If it is imperative to switch to flex_array_size() to quiesce
> checker warnings, please add a comment in the code explaining that the
> size evaluates to nseg_new-1 sge_ieee1212 structs.

Done:
	https://lore.kernel.org/lkml/20210413054032.GA276102@embeddedor/

Thanks!
--
Gustavo

Re: [PATCH][next] scsi: aacraid: Replace one-element array with flexible-array member

[James Bottomley]

On Tue, 2021-04-13 at 00:45 -0500, Gustavo A. R. Silva wrote:
> Hi Martin,
> 
> On 4/12/21 23:52, Martin K. Petersen wrote:
> 
> > Silencing analyzer warnings shouldn't be done at the expense of
> > human
> > readers. If it is imperative to switch to flex_array_size() to
> > quiesce
> > checker warnings, please add a comment in the code explaining that
> > the
> > size evaluates to nseg_new-1 sge_ieee1212 structs.
> 
> Done:
> 	
> https://lore.kernel.org/lkml/20210413054032.GA276102@embeddedor/

I think the reason everyone gets confused is that they think the first
argument should do something.  If flex_array_size had been defined

#define flex_array_size(p, count)			\
	array_size(count,				\
		    sizeof(*(p)) + __must_be_array(p))

Then we could have used

flex_array_size(sge, nseg_new - 1)

or

flex_array_size(rio->sge, nseg_new - 1)

and everyone would have understood either expression.  This would also
have been useful, as the first example demonstrates, when we have a
pointer rather than a flexible member ... although that means the macro
likely needs a new name.

However, perhaps just do

array_size(nseg_new - 1, sizeof(*sge));

And lose the comment?

James