Messages in this thread | | | Subject | Re: [PATCH] mctp.7: Add man page for Linux MCTP support | From | "Alejandro Colomar (man-pages)" <> | Date | Mon, 18 Oct 2021 08:59:07 +0200 |
| |
[CC += checkpatch.pl maintainers (see reason below)]
Hi Jeremy,
On 10/18/21 7:05 AM, Jeremy Kerr wrote: > Hi Alex, > >> Thanks for the manual page! > > And thanks for the review! In general, I've updated to suit your > comments, just a couple of queries inline. > >>> +.SH SYNOPSIS >>> +.nf >>> +.B #include <sys/socket.h> >>> +.B #include <linux/mctp.h> >>> +.PP >>> +.B mctp_socket = socket(AF_MCTP, SOCK_DGRAM, 0); >> >> mctp_socket is a variable name. See socket.7 for an example. >> It should be in italics. > > This was based on udp.7; want me to send a patch for that too?
Sure. Thanks!
> >>> +Packets between a local and remote endpoint are identified by the >>> source >>> +and destination EIDs, plus a three-bit tag value. >>> +.PP >>> +Addressing data is passed in socket system calls through >>> +.B struct sockaddr\_mctp >> >> That escape is unnecessary. Did you see it in another page perhaps? > > I thought I'd seen some odd line-breaks at the underscore, but can't > replicate that now. Will remove. > >>> +typedef uint8_t mctp_eid_t; >>> + >>> +struct mctp_addr { >>> + mctp_eid_t s_addr; >>> +}; >>> + >>> +struct sockaddr_mctp { >>> + unsigned short int smctp_family; /* = AF_MCTP */ >> >> We only use 'int' in 'unsigned int', as the kernel does (or attempts >> to do). checkpatch.pl warns about 'unsigned short int', IIRC. > > No, there are no warnings from checkpatch there; that's just copied from > the current kernel header.
Huh! That's weird; 'unsigned long int' does, so I expected the same with 'short'. Maybe a bug in checkpatch?
WARNING:UNNECESSARY_INT: Prefer 'unsigned long' over 'unsigned long int' as the int is unnecessary #42: FILE: /home/user/src/alx/test/unsigned_short_int.c:42: + unsigned long int a;
total: 0 errors, 1 warnings, 0 checks, 65 lines checked
> > However, I have just sent a separate patch to change that to > __kernel_sa_family_t. Should I use that here (keeping this an exact > match of the kernel header), or stick to the more familiar unsigned > short?
I prefer 'unsigned short' for consistency with 'unsigned long'.
> > Cheers, > > > Jeremy >
Cheers,
Alex
-- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
| |