Hi Jeremy,
On 11/18/21 06:08, Jeremy Kerr wrote:
Hi Alex,
I didn't even know there was such a convention, but if there is, yes,
I explicitly want to override it.
OK, I'll update to suit.
man-pages(7) recommends breaking long lines at clause boundaries
(commas, semicolons, and so on), but somethimes clauses (if you don't
know the difference between phrases and clauses, which you don't need
to, basically clauses are made up of phrases) are too long, and you
can have a single clause that uses more than a single line. In those
cases, the most sensible place to break the line is at the next level:
phrase boundaries.
OK, understood, thanks for that.
What I mean is, if in the future this structure will have additional
trailing fields, documenting this padding is unnecessary, since that
may
vary. Code should not rely on this structure having _only_ that
padding. And if code handles any arbitrary extra stuff in this
structure, it will implicitly also handle that __smctp_pad1 field, so
there's no need to mention it.
Example:
struct sockaddr_mctp {
unsigned short smctp_family; /* = AF_MCTP */
uint16_t __smctp_pad0; /* pad, must be zero */
int smctp_network; /* local network identifier */
struct mctp_addr smctp_addr; /* EID */
uint8_t smctp_type; /* message type byte */
uint8_t smctp_tag; /* tag value, including TO flag */
uint8_t foo; /* was __smctp_pad1 */
uint8_t bar; /* extra stuff */
};
Here I got rid of the pad, and even added an extra field.
Right, but you've also broken the ABI if that previous padding byte
wasn't explicitly present, and required to be zero.
Ahh, that's why I require in such cases a memset(p, 0, sizeof(*p));
That covers any undocumented padding; present or future.
In that future ABI
implementation, the kernel can't distinguish between 'foo' being properly
initialised, and not just random stack garbage from an old-ABI caller.
That's why we have the _pad1 field, and why we require it to be zero.
Since that's enforced by the kernel, I'd rather have it documented,
rather than users seeing their calls fail for "invisible" reasons, when
a call's _pad1 happens to contain a non-zero byte due to not being
initialised.
Code should be written to be compatible with this case, right?
I'm not 100% clear on you mean by compatible there - you want to prevent
the case where __smctp_pad1 is removed from the header, and that code is
now referencing an invalid struct member?
That's somewhat unavoidable, and also applies to _pad0; I'm not sure why
_pad1 needs to be different.
I want that programmers zero the structure not by p->_pad0 = 0,
but by bzero(3) or memset(3). That's how it covers any ammount of padding.
Nevertheless, I don't have a strong feeling about this, and if you
prefer to document the padding, I'm fine with that.
Cheers!
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
