Discussion:
style change for syscalls.master
Brooks Davis
2018-10-09 22:18:26 UTC
Permalink
I have a patch (https://reviews.freebsd.org/D17488) that removes the
need for backslash line continuations in syscalls.master files with
the aim of making the files more readable. Based on the functionality
in that patch, I'd like to propose changing the style of the file to
something better suited the long lines introduced by long type/variable
names and SAL annotations. I'd also like it to be easier to diff
between the main syscalls.master and the freebsd32 version. Longer
term, I hope to be able to generate freebsd32 files from the default
syscalls.master which will be aided by reducing diffs and making room
for more annotations.

To those ends, I propose a reformat along the lines of the examples below:

Old format:
1 AUE_EXIT STD { void sys_exit(int rval); } exit \
sys_exit_args void
...
5 AUE_OPEN_RWTC STD { int open( \
_In_z_ char *path, \
int flags, \
int mode); }

New format:
1 AUE_EXIT STD {
void sys_exit(
int rval
);
} exit sys_exit_args void
...
5 AUE_OPEN_RWTC STD {
int open(
_In_z_ char *path,
int flags,
int mode
);
}

For an example of where this makes diffs easier, consider preadv:

Old format:
289 AUE_PREADV STD { ssize_t preadv(int fd, \
_In_reads_(iovcnt) \
struct iovec *iovp, \
u_int iovcnt, off_t offset); }

New format:
289 AUE_PREADV STD {
ssize_t preadv(
int fd,
_In_reads_(iovcnt) struct iovec *iovp,
u_int iovcnt,
off_t offset
);
}

New format (freebsd32):
289 AUE_PREADV STD {
ssize_t freebsd32_preadv(
int fd,
_In_reads_(iovcnt) struct iovec32 *iovp,
u_int iovcnt,
uint32_t offset1,
uint32_t offset2
);
}


Some comments on this:
- One-variable-per-line allows for annotations and is struct-like which
is appropriate since this is more about declaring uap structs then
about function declarations.
- I chose the top level indent keep things readable without adding a
blank between each syscall block. I'd be happy to drop things back
one tab if we'd rather have blanks between each declaration.
- If one really cared about vertical space, one could combine the ");",
"}", and alternative names, but I didn't find that very aesthetic.

I'd like to know if this would break any out-of-tree parsers. If so,
we can easily keep backslashes, but I'd rather clear those out if
we're going to complicate svn blame for every line in the file. Parsers
written in more sensible languages than the in-tree sh+sed+awk monster
seem likely to be easy to fix and I hope that a more stylized format
will be easier to handle with dumb parsers, but I'd like to hear if this
change would cause issues.

-- Brooks

P.S. I'd plan to make this change after the 12.0 branch, but would like
to reach concensus sooner as I'm working on yet-another-syscall vector
in my work tree and I'd like to use this there.
Brooks Davis
2018-10-30 19:16:05 UTC
Permalink
I've posted a review and plan to commit tomorrow as this is blocking
further cleanups.

https://reviews.freebsd.org/D17706

-- Brooks
Post by Brooks Davis
I have a patch (https://reviews.freebsd.org/D17488) that removes the
need for backslash line continuations in syscalls.master files with
the aim of making the files more readable. Based on the functionality
in that patch, I'd like to propose changing the style of the file to
something better suited the long lines introduced by long type/variable
names and SAL annotations. I'd also like it to be easier to diff
between the main syscalls.master and the freebsd32 version. Longer
term, I hope to be able to generate freebsd32 files from the default
syscalls.master which will be aided by reducing diffs and making room
for more annotations.
1 AUE_EXIT STD { void sys_exit(int rval); } exit \
sys_exit_args void
...
5 AUE_OPEN_RWTC STD { int open( \
_In_z_ char *path, \
int flags, \
int mode); }
1 AUE_EXIT STD {
void sys_exit(
int rval
);
} exit sys_exit_args void
...
5 AUE_OPEN_RWTC STD {
int open(
_In_z_ char *path,
int flags,
int mode
);
}
289 AUE_PREADV STD { ssize_t preadv(int fd, \
_In_reads_(iovcnt) \
struct iovec *iovp, \
u_int iovcnt, off_t offset); }
289 AUE_PREADV STD {
ssize_t preadv(
int fd,
_In_reads_(iovcnt) struct iovec *iovp,
u_int iovcnt,
off_t offset
);
}
289 AUE_PREADV STD {
ssize_t freebsd32_preadv(
int fd,
_In_reads_(iovcnt) struct iovec32 *iovp,
u_int iovcnt,
uint32_t offset1,
uint32_t offset2
);
}
- One-variable-per-line allows for annotations and is struct-like which
is appropriate since this is more about declaring uap structs then
about function declarations.
- I chose the top level indent keep things readable without adding a
blank between each syscall block. I'd be happy to drop things back
one tab if we'd rather have blanks between each declaration.
- If one really cared about vertical space, one could combine the ");",
"}", and alternative names, but I didn't find that very aesthetic.
I'd like to know if this would break any out-of-tree parsers. If so,
we can easily keep backslashes, but I'd rather clear those out if
we're going to complicate svn blame for every line in the file. Parsers
written in more sensible languages than the in-tree sh+sed+awk monster
seem likely to be easy to fix and I hope that a more stylized format
will be easier to handle with dumb parsers, but I'd like to hear if this
change would cause issues.
-- Brooks
P.S. I'd plan to make this change after the 12.0 branch, but would like
to reach concensus sooner as I'm working on yet-another-syscall vector
in my work tree and I'd like to use this there.
Loading...