2019-09-27 16:59:27

by Stephen Kitt

[permalink] [raw]
Subject: [PATCH] docs: use flexible array members, not zero-length

Update the docs throughout to remove zero-length arrays, replacing
them with C99 flexible array members. GCC will then ensure that the
arrays are always the last element in the struct.

Signed-off-by: Stephen Kitt <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
---
Documentation/bpf/btf.rst | 2 +-
Documentation/digsig.txt | 4 ++--
Documentation/driver-api/connector.rst | 2 +-
Documentation/driver-api/usb/URB.rst | 2 +-
.../filesystems/autofs-mount-control.txt | 2 +-
Documentation/filesystems/autofs.txt | 2 +-
Documentation/filesystems/fiemap.txt | 2 +-
Documentation/hwspinlock.txt | 2 +-
Documentation/networking/can.rst | 2 +-
Documentation/networking/rxrpc.txt | 2 +-
Documentation/remoteproc.txt | 4 ++--
Documentation/virt/kvm/api.txt | 24 +++++++++----------
Documentation/x86/boot.rst | 4 ++--
13 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 4d565d202ce3..24ce50fc1fc1 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -670,7 +670,7 @@ func_info for each specific ELF section.::
__u32 sec_name_off; /* offset to section name */
__u32 num_info;
/* Followed by num_info * record_size number of bytes */
- __u8 data[0];
+ __u8 data[];
};

Here, num_info must be greater than 0.
diff --git a/Documentation/digsig.txt b/Documentation/digsig.txt
index f6a8902d3ef7..d7479f98a27e 100644
--- a/Documentation/digsig.txt
+++ b/Documentation/digsig.txt
@@ -31,7 +31,7 @@ Public key and signature consist of header and MPIs::
time_t timestamp; /* key made, always 0 for now */
uint8_t algo;
uint8_t nmpi;
- char mpi[0];
+ char mpi[];
} __packed;

struct signature_hdr {
@@ -41,7 +41,7 @@ Public key and signature consist of header and MPIs::
uint8_t hash;
uint8_t keyid[8];
uint8_t nmpi;
- char mpi[0];
+ char mpi[];
} __packed;

keyid equals to SHA1[12-19] over the total key content.
diff --git a/Documentation/driver-api/connector.rst b/Documentation/driver-api/connector.rst
index c100c7482289..1f187028c7e1 100644
--- a/Documentation/driver-api/connector.rst
+++ b/Documentation/driver-api/connector.rst
@@ -49,7 +49,7 @@ be dereferenced to `struct cn_msg *`::
__u32 ack;

__u32 len; /* Length of the following data */
- __u8 data[0];
+ __u8 data[];
};

Connector interfaces
diff --git a/Documentation/driver-api/usb/URB.rst b/Documentation/driver-api/usb/URB.rst
index 61a54da9fce9..8aca9c39e9a0 100644
--- a/Documentation/driver-api/usb/URB.rst
+++ b/Documentation/driver-api/usb/URB.rst
@@ -82,7 +82,7 @@ Some of the fields in struct :c:type:`urb` are::

// ISO only: packets are only "best effort"; each can have errors
int error_count; // number of errors
- struct usb_iso_packet_descriptor iso_frame_desc[0];
+ struct usb_iso_packet_descriptor iso_frame_desc[];
};

Your driver must create the "pipe" value using values from the appropriate
diff --git a/Documentation/filesystems/autofs-mount-control.txt b/Documentation/filesystems/autofs-mount-control.txt
index acc02fc57993..1c5cf1ecd90d 100644
--- a/Documentation/filesystems/autofs-mount-control.txt
+++ b/Documentation/filesystems/autofs-mount-control.txt
@@ -194,7 +194,7 @@ struct autofs_dev_ioctl {
struct args_ismountpoint ismountpoint;
};

- char path[0];
+ char path[];
};

The ioctlfd field is a mount point file descriptor of an autofs mount
diff --git a/Documentation/filesystems/autofs.txt b/Documentation/filesystems/autofs.txt
index 3af38c7fd26d..0766089b81f1 100644
--- a/Documentation/filesystems/autofs.txt
+++ b/Documentation/filesystems/autofs.txt
@@ -455,7 +455,7 @@ Each ioctl is passed a pointer to an `autofs_dev_ioctl` structure:
struct args_ismountpoint ismountpoint;
};

- char path[0];
+ char path[];
};

For the **OPEN_MOUNT** and **IS_MOUNTPOINT** commands, the target
diff --git a/Documentation/filesystems/fiemap.txt b/Documentation/filesystems/fiemap.txt
index f6d9c99103a4..172c94377fb3 100644
--- a/Documentation/filesystems/fiemap.txt
+++ b/Documentation/filesystems/fiemap.txt
@@ -22,7 +22,7 @@ struct fiemap {
* mapped (out) */
__u32 fm_extent_count; /* size of fm_extents array (in) */
__u32 fm_reserved;
- struct fiemap_extent fm_extents[0]; /* array of mapped extents (out) */
+ struct fiemap_extent fm_extents[]; /* array of mapped extents (out) */
};


diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt
index 6f03713b7003..a37e7c24ff26 100644
--- a/Documentation/hwspinlock.txt
+++ b/Documentation/hwspinlock.txt
@@ -439,7 +439,7 @@ implementation using the hwspin_lock_register() API.
const struct hwspinlock_ops *ops;
int base_id;
int num_locks;
- struct hwspinlock lock[0];
+ struct hwspinlock lock[];
};

struct hwspinlock_device contains an array of hwspinlock structs, each
diff --git a/Documentation/networking/can.rst b/Documentation/networking/can.rst
index 2fd0b51a8c52..ceb40a9d2044 100644
--- a/Documentation/networking/can.rst
+++ b/Documentation/networking/can.rst
@@ -705,7 +705,7 @@ The broadcast manager sends responses to user space in the same form:
struct timeval ival1, ival2; /* count and subsequent interval */
canid_t can_id; /* unique can_id for task */
__u32 nframes; /* number of can_frames following */
- struct can_frame frames[0];
+ struct can_frame frames[];
};

The aligned payload 'frames' uses the same basic CAN frame structure defined
diff --git a/Documentation/networking/rxrpc.txt b/Documentation/networking/rxrpc.txt
index 180e07d956a7..b1fff6870c1b 100644
--- a/Documentation/networking/rxrpc.txt
+++ b/Documentation/networking/rxrpc.txt
@@ -518,7 +518,7 @@ form:
uint8_t kvno; /* key version number */
uint8_t __pad[3];
uint8_t session_key[8]; /* DES session key */
- uint8_t ticket[0]; /* the encrypted ticket */
+ uint8_t ticket[]; /* the encrypted ticket */
};

Where the ticket blob is just appended to the above structure.
diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt
index 03c3d2e568b0..77b1493759b2 100644
--- a/Documentation/remoteproc.txt
+++ b/Documentation/remoteproc.txt
@@ -274,7 +274,7 @@ The resource table begins with this header::
u32 ver;
u32 num;
u32 reserved[2];
- u32 offset[0];
+ u32 offset[];
} __packed;

Immediately following this header are the resource entries themselves,
@@ -291,7 +291,7 @@ each of which begins with the following resource entry header::
*/
struct fw_rsc_hdr {
u32 type;
- u8 data[0];
+ u8 data[];
} __packed;

Some resources entries are mere announcements, where the host is informed
diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt
index 136f1eef3712..0b8955152362 100644
--- a/Documentation/virt/kvm/api.txt
+++ b/Documentation/virt/kvm/api.txt
@@ -192,7 +192,7 @@ Errors:

struct kvm_msr_list {
__u32 nmsrs; /* number of msrs in entries */
- __u32 indices[0];
+ __u32 indices[];
};

The user fills in the size of the indices array in nmsrs, and in return
@@ -566,7 +566,7 @@ struct kvm_msrs {
__u32 nmsrs; /* number of msrs in entries */
__u32 pad;

- struct kvm_msr_entry entries[0];
+ struct kvm_msr_entry entries[];
};

struct kvm_msr_entry {
@@ -626,7 +626,7 @@ struct kvm_cpuid_entry {
struct kvm_cpuid {
__u32 nent;
__u32 padding;
- struct kvm_cpuid_entry entries[0];
+ struct kvm_cpuid_entry entries[];
};


@@ -649,7 +649,7 @@ signal mask.
/* for KVM_SET_SIGNAL_MASK */
struct kvm_signal_mask {
__u32 len;
- __u8 sigset[0];
+ __u8 sigset[];
};


@@ -1403,7 +1403,7 @@ Returns: 0 on success, -1 on error
struct kvm_cpuid2 {
__u32 nent;
__u32 padding;
- struct kvm_cpuid_entry2 entries[0];
+ struct kvm_cpuid_entry2 entries[];
};

#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX BIT(0)
@@ -1516,7 +1516,7 @@ On arm/arm64, GSI routing has the following limitation:
struct kvm_irq_routing {
__u32 nr;
__u32 flags;
- struct kvm_irq_routing_entry entries[0];
+ struct kvm_irq_routing_entry entries[];
};

No flags are specified so far, the corresponding field must be set to zero.
@@ -2868,7 +2868,7 @@ Errors:

struct kvm_reg_list {
__u64 n; /* number of registers in reg[] */
- __u64 reg[0];
+ __u64 reg[];
};

This ioctl returns the guest registers that are supported for the
@@ -2997,7 +2997,7 @@ Returns: 0 on success, -1 on error
struct kvm_cpuid2 {
__u32 nent;
__u32 flags;
- struct kvm_cpuid_entry2 entries[0];
+ struct kvm_cpuid_entry2 entries[];
};

The member 'flags' is used for passing flags from userspace.
@@ -3889,8 +3889,8 @@ struct kvm_nested_state {
} hdr;

union {
- struct kvm_vmx_nested_state_data vmx[0];
- struct kvm_svm_nested_state_data svm[0];
+ struct kvm_vmx_nested_state_data vmx[];
+ struct kvm_svm_nested_state_data svm[];
} data;
};

@@ -4016,7 +4016,7 @@ Returns: 0 on success, -1 on error
struct kvm_cpuid2 {
__u32 nent;
__u32 padding;
- struct kvm_cpuid_entry2 entries[0];
+ struct kvm_cpuid_entry2 entries[];
};

struct kvm_cpuid_entry2 {
@@ -4110,7 +4110,7 @@ struct kvm_pmu_event_filter {
__u32 fixed_counter_bitmap;
__u32 flags;
__u32 pad[4];
- __u64 events[0];
+ __u64 events[];
};

This ioctl restricts the set of PMU events that the guest can program.
diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f100c0e6..1110c1a7337b 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -790,13 +790,13 @@ Protocol: 2.09+
The 64-bit physical pointer to NULL terminated single linked list of
struct setup_data. This is used to define a more extensible boot
parameters passing mechanism. The definition of struct setup_data is
- as follow::
+ as follows::

struct setup_data {
u64 next;
u32 type;
u32 len;
- u8 data[0];
+ u8 data[];
};

Where, the next is a 64-bit physical pointer to the next node of
--
2.20.1


2019-09-28 07:17:29

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] docs: use flexible array members, not zero-length

On Fri, 27 Sep 2019 16:29:27 +0200
Stephen Kitt <[email protected]> wrote:

> Update the docs throughout to remove zero-length arrays, replacing
> them with C99 flexible array members. GCC will then ensure that the
> arrays are always the last element in the struct.

I appreciate the thought but...

> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index 4d565d202ce3..24ce50fc1fc1 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst
> @@ -670,7 +670,7 @@ func_info for each specific ELF section.::
> __u32 sec_name_off; /* offset to section name */
> __u32 num_info;
> /* Followed by num_info * record_size number of bytes */
> - __u8 data[0];
> + __u8 data[];
> };

I only checked this one, but found what I had expected: the actual
definition of this structure (found in tools/lib/bpf/libbpf_internal.h)
says "data[0]". We can't really make the documentation read the way we
*wish* the source would be, we need to document reality.

I'm pretty sure that most of the other examples will be the same.

If you really want to fix these, the right solution is to fix the offending
structures — one patch per structure — in the source, then update the
documentation to match the new reality.

Thanks,

jon

2019-09-28 12:34:13

by Stephen Kitt

[permalink] [raw]
Subject: Re: [PATCH] docs: use flexible array members, not zero-length

On Sat, 28 Sep 2019 01:16:39 -0600, Jonathan Corbet <[email protected]> wrote:
> On Fri, 27 Sep 2019 16:29:27 +0200
> Stephen Kitt <[email protected]> wrote:
> > diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> > index 4d565d202ce3..24ce50fc1fc1 100644
> > --- a/Documentation/bpf/btf.rst
> > +++ b/Documentation/bpf/btf.rst
> > @@ -670,7 +670,7 @@ func_info for each specific ELF section.::
> > __u32 sec_name_off; /* offset to section name */
> > __u32 num_info;
> > /* Followed by num_info * record_size number of bytes */
> > - __u8 data[0];
> > + __u8 data[];
> > };
>
> I only checked this one, but found what I had expected: the actual
> definition of this structure (found in tools/lib/bpf/libbpf_internal.h)
> says "data[0]". We can't really make the documentation read the way we
> *wish* the source would be, we need to document reality.
>
> I'm pretty sure that most of the other examples will be the same.

Aargh, yes, of course, thanks for checking! I was locked in a “prescriptive”
documentation mode, but this type of documentation has to be descriptive
since it’s documenting shared structures, not structures which developers
have to write.

> If you really want to fix these, the right solution is to fix the offending
> structures — one patch per structure — in the source, then update the
> documentation to match the new reality.

Yes. I have a Coccinelle script which takes care of the code, but it doesn’t
work for docs ;-).

Wouldn’t it be better to update the docs simultaneously in each patch which
fixes a structure? Or is that unworkable with current development practices?

Regards,

Stephen


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature

2019-09-30 20:41:45

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] docs: use flexible array members, not zero-length

On Sat, 28 Sep 2019 10:55:57 +0200
Stephen Kitt <[email protected]> wrote:

> Wouldn’t it be better to update the docs simultaneously in each patch which
> fixes a structure? Or is that unworkable with current development practices?

Definitely update the two together. The doc fix should just go through
the appropriate maintainer with the code change.

Thanks,

jon