Hi Michael,
I have a lot of patches here.
Some of them are trivial, and some of them are not.
I have them sorted by their contents more or less, but if you
prefer completely unrelated email threads for completely unrelated
patches, please tell me.
Cheers,
Alex
Alejandro Colomar (24):
* printf():
inet_net_pton.3: Use 'PRIx32' rather than "%x" when printing
'uint32_t' values
endian.3: Use 'PRIx32' rather than "%x" when printing 'uint32_t'
values
timerfd_create.2: Use 'PRIxN' macros when printing C99 fixed-width
integer types
eventfd.2: Use 'PRIxN' macros when printing C99 fixed-width integer
types
offsetof.3: Use "%zu" rather than "%zd" when printing 'size_t' values
timer_create.2: Cast to 'unsigned long' rathen than 'long' when
printing with "%lx"
request_key.2: Cast to 'unsigned long' rather than 'long' when
printing with "%lx"
add_key.2: Cast to 'unsigned long' rather than 'long' when printing
with "%lx"
* casts:
clock_getcpuclockid.3: Remove unneeded cast
ioctl_ns.2: Remove unneeded cast
stat.2: Remove unneeded cast
* sizeof():
getgrent_r.3: Use sizeof() to get buffer size (instead of hardcoding
macro name)
getpwent_r.3: Use sizeof() to get buffer size (instead of hardcoding
macro name)
fread.3: Move ARRAY_SIZE logic into macro
unix.7: Use sizeof() to get buffer size (instead of hardcoding macro
name)
* types:
getpwent_r.3: Declare variables with different types in different
lines
get_phys_pages.3: Write 'long' instead of 'long int'
core.5: Use adequate type
* trivial patches
pthread_setname_np.3: ffix
loop.4: ffix
* other:
aio.7: Use perror() directly
membarrier.2: Note that glibc does not provide a wrapper
select_tut.2: Use MAX(a, b) from <sys/param.h>
bpf.2: Add missing headers
man2/add_key.2 | 2 +-
man2/bpf.2 | 12 ++++++++++++
man2/eventfd.2 | 4 ++--
man2/ioctl_ns.2 | 10 ++++------
man2/membarrier.2 | 9 +++++++++
man2/request_key.2 | 2 +-
man2/select_tut.2 | 10 ++++------
man2/stat.2 | 4 ++--
man2/timer_create.2 | 4 ++--
man2/timerfd_create.2 | 5 ++---
man3/clock_getcpuclockid.3 | 2 +-
man3/endian.3 | 7 ++++---
man3/fread.3 | 6 +++---
man3/get_phys_pages.3 | 4 ++--
man3/getgrent_r.3 | 2 +-
man3/getpwent_r.3 | 5 +++--
man3/inet_net_pton.3 | 3 ++-
man3/offsetof.3 | 4 ++--
man3/pthread_setname_np.3 | 5 +++--
man4/loop.4 | 2 +-
man5/core.5 | 5 ++---
man7/aio.7 | 6 ++----
man7/unix.7 | 14 +++++++-------
23 files changed, 72 insertions(+), 55 deletions(-)
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/eventfd.2 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man2/eventfd.2 b/man2/eventfd.2
index 929234ab7..71e9d85b4 100644
--- a/man2/eventfd.2
+++ b/man2/eventfd.2
@@ -386,6 +386,7 @@ Parent read 28 (0x1c) from efd
.EX
#include <sys/eventfd.h>
#include <unistd.h>
+#include <inttypes.h> /* Definition of PRIu64 & PRIx64 */
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h> /* Definition of uint64_t */
@@ -430,8 +431,7 @@ main(int argc, char *argv[])
s = read(efd, &u, sizeof(uint64_t));
if (s != sizeof(uint64_t))
handle_error("read");
- printf("Parent read %llu (0x%llx) from efd\en",
- (unsigned long long) u, (unsigned long long) u);
+ printf("Parent read %"PRIu64" (0x%"PRIx64") from efd\en", u, u);
exit(EXIT_SUCCESS);
case \-1:
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/offsetof.3 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man3/offsetof.3 b/man3/offsetof.3
index 05fdd6dd8..f0c9975f4 100644
--- a/man3/offsetof.3
+++ b/man3/offsetof.3
@@ -93,10 +93,10 @@ main(void)
/* Output is compiler dependent */
- printf("offsets: i=%zd; c=%zd; d=%zd a=%zd\en",
+ printf("offsets: i=%zu; c=%zu; d=%zu a=%zu\en",
offsetof(struct s, i), offsetof(struct s, c),
offsetof(struct s, d), offsetof(struct s, a));
- printf("sizeof(struct s)=%zd\en", sizeof(struct s));
+ printf("sizeof(struct s)=%zu\en", sizeof(struct s));
exit(EXIT_SUCCESS);
}
--
2.28.0
Both major(3) and minor(3) return an 'unsigned int',
so there is no need to use a 'long' for printing.
Moreover, it should have been 'unsigned long',
as "%lx" expects an unsigned type.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/ioctl_ns.2 | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/man2/ioctl_ns.2 b/man2/ioctl_ns.2
index 818dde32c..bf832a2c7 100644
--- a/man2/ioctl_ns.2
+++ b/man2/ioctl_ns.2
@@ -316,9 +316,8 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
printf("Device/Inode of owning user namespace is: "
- "[%lx,%lx] / %ld\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev),
- (long) sb.st_ino);
+ "[%x,%x] / %ld\en",
+ major(sb.st_dev), minor(sb.st_dev), (long) sb.st_ino);
close(userns_fd);
}
@@ -345,9 +344,8 @@ main(int argc, char *argv[])
perror("fstat\-parentns");
exit(EXIT_FAILURE);
}
- printf("Device/Inode of parent namespace is: [%lx,%lx] / %ld\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev),
- (long) sb.st_ino);
+ printf("Device/Inode of parent namespace is: [%x,%x] / %ld\en",
+ major(sb.st_dev), minor(sb.st_dev), (long) sb.st_ino);
close(parent_fd);
}
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/inet_net_pton.3 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/man3/inet_net_pton.3 b/man3/inet_net_pton.3
index 00f94b9d4..d74a33d74 100644
--- a/man3/inet_net_pton.3
+++ b/man3/inet_net_pton.3
@@ -332,6 +332,7 @@ Raw address: c1a80180
/* Link with "\-lresolv" */
#include <arpa/inet.h>
+#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
@@ -381,7 +382,7 @@ main(int argc, char *argv[])
may not have been touched by inet_net_ntop(), and so will still
have any initial value that was specified in argv[2]. */
- printf("Raw address: %x\en", htonl(addr.s_addr));
+ printf("Raw address: %"PRIx32"\en", htonl(addr.s_addr));
exit(EXIT_SUCCESS);
}
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/fread.3 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/man3/fread.3 b/man3/fread.3
index 4c5dc3dbc..8e71e620e 100644
--- a/man3/fread.3
+++ b/man3/fread.3
@@ -136,6 +136,8 @@ Class: 0x02
#include <stdio.h>
#include <stdlib.h>
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
int
main(void)
{
@@ -147,9 +149,7 @@ main(void)
unsigned char buffer[4];
- size_t ret =
- fread(buffer, sizeof(buffer) / sizeof(*buffer), sizeof(*buffer),
- fp);
+ size_t ret = fread(buffer, ARRAY_SIZE(buffer), sizeof(*buffer), fp);
if (ret != sizeof(*buffer)) {
fprintf(stderr, "fread() failed: %zu\en", ret);
exit(EXIT_FAILURE);
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/endian.3 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/man3/endian.3 b/man3/endian.3
index bdf1efd7e..3a898bb02 100644
--- a/man3/endian.3
+++ b/man3/endian.3
@@ -147,6 +147,7 @@ htobe32(x.u32) = 0x11223344
\&
.EX
#include <endian.h>
+#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
@@ -164,9 +165,9 @@ main(int argc, char *argv[])
x.arr[2] = 0x33;
x.arr[3] = 0x44; /* Highest-address byte */
- printf("x.u32 = 0x%x\en", x.u32);
- printf("htole32(x.u32) = 0x%x\en", htole32(x.u32));
- printf("htobe32(x.u32) = 0x%x\en", htobe32(x.u32));
+ printf("x.u32 = 0x%"PRIx32"\en", x.u32);
+ printf("htole32(x.u32) = 0x%"PRIx32"\en", htole32(x.u32));
+ printf("htobe32(x.u32) = 0x%"PRIx32"\en", htobe32(x.u32));
exit(EXIT_SUCCESS);
}
--
2.28.0
I added some headers to reduce the number of warnings.
I found the needed headers by using grep, but maybe some of them
shouldn't be included directly.
The example still has many problems to compile.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/bpf.2 | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/man2/bpf.2 b/man2/bpf.2
index b45acde76..d26d6a43d 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -981,6 +981,18 @@ ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
* 3. attach prog_fd to raw socket via setsockopt()
* 4. print number of received TCP/UDP packets every second
*/
+#include <assert.h>
+#include <errno.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+
int
main(int argc, char **argv)
{
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man4/loop.4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man4/loop.4 b/man4/loop.4
index 73b23871d..1b5d05666 100644
--- a/man4/loop.4
+++ b/man4/loop.4
@@ -227,7 +227,7 @@ in
.IR loop_config.info.lo_flags ;
and
.IP *
-Explicitly request read-only mode by setting
+explicitly request read-only mode by setting
.BR LO_FLAGS_READ_ONLY
in
.IR loop_config.info.lo_flags .
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man7/unix.7 | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/man7/unix.7 b/man7/unix.7
index f61b51424..50828a5bc 100644
--- a/man7/unix.7
+++ b/man7/unix.7
@@ -991,7 +991,7 @@ main(int argc, char *argv[])
/* Wait for next data packet. */
- ret = read(data_socket, buffer, BUFFER_SIZE);
+ ret = read(data_socket, buffer, sizeof(buffer));
if (ret == \-1) {
perror("read");
exit(EXIT_FAILURE);
@@ -999,16 +999,16 @@ main(int argc, char *argv[])
/* Ensure buffer is 0\-terminated. */
- buffer[BUFFER_SIZE \- 1] = 0;
+ buffer[sizeof(buffer) \- 1] = 0;
/* Handle commands. */
- if (!strncmp(buffer, "DOWN", BUFFER_SIZE)) {
+ if (!strncmp(buffer, "DOWN", sizeof(buffer))) {
down_flag = 1;
break;
}
- if (!strncmp(buffer, "END", BUFFER_SIZE)) {
+ if (!strncmp(buffer, "END", sizeof(buffer))) {
break;
}
@@ -1020,7 +1020,7 @@ main(int argc, char *argv[])
/* Send result. */
sprintf(buffer, "%d", result);
- ret = write(data_socket, buffer, BUFFER_SIZE);
+ ret = write(data_socket, buffer, sizeof(buffer));
if (ret == \-1) {
perror("write");
exit(EXIT_FAILURE);
@@ -1116,7 +1116,7 @@ main(int argc, char *argv[])
/* Receive result. */
- ret = read(data_socket, buffer, BUFFER_SIZE);
+ ret = read(data_socket, buffer, sizeof(buffer));
if (ret == \-1) {
perror("read");
exit(EXIT_FAILURE);
@@ -1124,7 +1124,7 @@ main(int argc, char *argv[])
/* Ensure buffer is 0\-terminated. */
- buffer[BUFFER_SIZE \- 1] = 0;
+ buffer[sizeof(buffer) \- 1] = 0;
printf("Result = %s\en", buffer);
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/getgrent_r.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
index 81d81a851..76deec370 100644
--- a/man3/getgrent_r.3
+++ b/man3/getgrent_r.3
@@ -186,7 +186,7 @@ main(void)
setgrent();
while (1) {
- i = getgrent_r(&grp, buf, BUFLEN, &grpp);
+ i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
if (i)
break;
printf("%s (%d):", grpp\->gr_name, grpp\->gr_gid);
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/getpwent_r.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man3/getpwent_r.3 b/man3/getpwent_r.3
index 64f27dbfb..0f7581091 100644
--- a/man3/getpwent_r.3
+++ b/man3/getpwent_r.3
@@ -190,7 +190,7 @@ main(void)
setpwent();
while (1) {
- i = getpwent_r(&pw, buf, BUFLEN, &pwp);
+ i = getpwent_r(&pw, buf, sizeof(buf), &pwp);
if (i)
break;
printf("%s (%d)\etHOME %s\etSHELL %s\en", pwp\->pw_name,
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/select_tut.2 | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/man2/select_tut.2 b/man2/select_tut.2
index f683dd49d..d23683d75 100644
--- a/man2/select_tut.2
+++ b/man2/select_tut.2
@@ -354,6 +354,7 @@ from one TCP port to another.
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
+#include <sys/param.h> /* Definition of MAX(a, b) */
#include <sys/select.h>
#include <string.h>
#include <signal.h>
@@ -364,9 +365,6 @@ from one TCP port to another.
static int forward_port;
-#undef max
-#define max(x,y) ((x) > (y) ? (x) : (y))
-
static int
listen_socket(int listen_port)
{
@@ -483,7 +481,7 @@ main(int argc, char *argv[])
FD_ZERO(&writefds);
FD_ZERO(&exceptfds);
FD_SET(h, &readfds);
- nfds = max(nfds, h);
+ nfds = MAX(nfds, h);
if (fd1 > 0 && buf1_avail < BUF_SIZE)
FD_SET(fd1, &readfds);
@@ -499,11 +497,11 @@ main(int argc, char *argv[])
if (fd1 > 0) {
FD_SET(fd1, &exceptfds);
- nfds = max(nfds, fd1);
+ nfds = MAX(nfds, fd1);
}
if (fd2 > 0) {
FD_SET(fd2, &exceptfds);
- nfds = max(nfds, fd2);
+ nfds = MAX(nfds, fd2);
}
ready = select(nfds + 1, &readfds, &writefds, &exceptfds, NULL);
--
2.28.0
Notes: I copied .nf and .fi from futex.2, but they made no visual difference.
What do they actually do?
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/membarrier.2 | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/man2/membarrier.2 b/man2/membarrier.2
index 8825de71e..f65c6be5c 100644
--- a/man2/membarrier.2
+++ b/man2/membarrier.2
@@ -26,9 +26,15 @@
.SH NAME
membarrier \- issue memory barriers on a set of threads
.SH SYNOPSIS
+.nf
+.PP
.B #include <linux/membarrier.h>
.PP
.BI "int membarrier(int " cmd ", int " flags ");"
+.fi
+.PP
+.IR Note :
+There is no glibc wrapper for this system call; see NOTES.
.SH DESCRIPTION
The
.BR membarrier ()
@@ -270,6 +276,9 @@ Examples where
.BR membarrier ()
can be useful include implementations
of Read-Copy-Update libraries and garbage collectors.
+.PP
+Glibc does not provide a wrapper for this system call; call it using
+.BR syscall (2).
.SH EXAMPLES
Assuming a multithreaded application where "fast_path()" is executed
very frequently, and where "slow_path()" is executed infrequently, the
--
2.28.0
'nread' is of type 'ssize_t'
'tot' adds up different values contained in 'nread',
so it should also be 'ssize_t', and not 'int' (which possibly overflows).
Signed-off-by: Alejandro Colomar <[email protected]>
---
man5/core.5 | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/man5/core.5 b/man5/core.5
index a87ebbaf4..45c9de845 100644
--- a/man5/core.5
+++ b/man5/core.5
@@ -654,8 +654,7 @@ Total bytes in core dump: 282624
int
main(int argc, char *argv[])
{
- int tot;
- ssize_t nread;
+ ssize_t nread, tot;
char buf[BUF_SIZE];
FILE *fp;
char cwd[PATH_MAX];
@@ -684,7 +683,7 @@ main(int argc, char *argv[])
tot = 0;
while ((nread = read(STDIN_FILENO, buf, BUF_SIZE)) > 0)
tot += nread;
- fprintf(fp, "Total bytes in core dump: %d\en", tot);
+ fprintf(fp, "Total bytes in core dump: %zd\en", tot);
fclose(fp);
exit(EXIT_SUCCESS);
--
2.28.0
For consistency.
Most man pages use 'long' instead of 'long int'.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/get_phys_pages.3 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man3/get_phys_pages.3 b/man3/get_phys_pages.3
index 4a9177dfd..97ba625b7 100644
--- a/man3/get_phys_pages.3
+++ b/man3/get_phys_pages.3
@@ -30,8 +30,8 @@ page counts
.nf
.B "#include <sys/sysinfo.h>"
.PP
-.B long int get_phys_pages(void);
-.B long int get_avphys_pages(void);
+.B long get_phys_pages(void);
+.B long get_avphys_pages(void);
.fi
.SH DESCRIPTION
The function
--
2.28.0
Both major(3) and minor(3) return an 'unsigned int',
so there is no need to use a 'long' for printing.
Moreover, it should have been 'unsigned long',
as "%lx" expects an unsigned type.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/stat.2 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man2/stat.2 b/man2/stat.2
index b35e3c615..3418e1b5d 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -664,8 +664,8 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
- printf("ID of containing device: [%lx,%lx]\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev));
+ printf("ID of containing device: [%x,%x]\en",
+ major(sb.st_dev), minor(sb.st_dev));
printf("File type: ");
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/getpwent_r.3 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/man3/getpwent_r.3 b/man3/getpwent_r.3
index 0f7581091..b6c1c281f 100644
--- a/man3/getpwent_r.3
+++ b/man3/getpwent_r.3
@@ -184,7 +184,8 @@ in the stream with all other threads.
int
main(void)
{
- struct passwd pw, *pwp;
+ struct passwd pw;
+ struct passwd *pwp;
char buf[BUFLEN];
int i;
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man7/aio.7 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/man7/aio.7 b/man7/aio.7
index ff099885e..9b2c44c48 100644
--- a/man7/aio.7
+++ b/man7/aio.7
@@ -257,8 +257,6 @@ aio_return():
#define errExit(msg) do { perror(msg); exit(EXIT_FAILURE); } while (0)
-#define errMsg(msg) do { perror(msg); } while (0)
-
struct ioRequest { /* Application\-defined structure for tracking
I/O requests */
int reqNum;
@@ -390,7 +388,7 @@ main(int argc, char *argv[])
else if (s == AIO_ALLDONE)
printf("I/O all done\en");
else
- errMsg("aio_cancel");
+ perror("aio_cancel");
}
}
@@ -418,7 +416,7 @@ main(int argc, char *argv[])
printf("Canceled\en");
break;
default:
- errMsg("aio_error");
+ perror("aio_error");
break;
}
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/request_key.2 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man2/request_key.2 b/man2/request_key.2
index e28c11ded..c14bcb000 100644
--- a/man2/request_key.2
+++ b/man2/request_key.2
@@ -537,7 +537,7 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
- printf("Key ID is %lx\en", (long) key);
+ printf("Key ID is %lx\en", (unsigned long) key);
exit(EXIT_SUCCESS);
}
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/add_key.2 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man2/add_key.2 b/man2/add_key.2
index bda08c3d2..bbd2d1d03 100644
--- a/man2/add_key.2
+++ b/man2/add_key.2
@@ -262,7 +262,7 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
- printf("Key ID is %lx\en", (long) key);
+ printf("Key ID is %lx\en", (unsigned long) key);
exit(EXIT_SUCCESS);
}
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/timerfd_create.2 | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/man2/timerfd_create.2 b/man2/timerfd_create.2
index 67a13dba3..90e35d9b5 100644
--- a/man2/timerfd_create.2
+++ b/man2/timerfd_create.2
@@ -624,6 +624,7 @@ a.out 3 1 100
#include <sys/timerfd.h>
#include <time.h>
#include <unistd.h>
+#include <inttypes.h> /* Definition of PRIu64 */
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h> /* Definition of uint64_t */
@@ -706,9 +707,7 @@ main(int argc, char *argv[])
tot_exp += exp;
print_elapsed_time();
- printf("read: %llu; total=%llu\en",
- (unsigned long long) exp,
- (unsigned long long) tot_exp);
+ printf("read: %"PRIu64"; total=%"PRIu64"\en", exp, tot_exp);
}
exit(EXIT_SUCCESS);
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/timer_create.2 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man2/timer_create.2 b/man2/timer_create.2
index e9a8b8503..9c9907739 100644
--- a/man2/timer_create.2
+++ b/man2/timer_create.2
@@ -390,7 +390,7 @@ print_siginfo(siginfo_t *si)
tidp = si\->si_value.sival_ptr;
printf(" sival_ptr = %p; ", si\->si_value.sival_ptr);
- printf(" *sival_ptr = 0x%lx\en", (long) *tidp);
+ printf(" *sival_ptr = 0x%lx\en", (unsigned long) *tidp);
or = timer_getoverrun(*tidp);
if (or == \-1)
@@ -454,7 +454,7 @@ main(int argc, char *argv[])
if (timer_create(CLOCKID, &sev, &timerid) == \-1)
errExit("timer_create");
- printf("timer ID is 0x%lx\en", (long) timerid);
+ printf("timer ID is 0x%lx\en", (unsigned long) timerid);
/* Start the timer */
--
2.28.0
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/pthread_setname_np.3 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/man3/pthread_setname_np.3 b/man3/pthread_setname_np.3
index b206f66c0..4dc4960cd 100644
--- a/man3/pthread_setname_np.3
+++ b/man3/pthread_setname_np.3
@@ -164,8 +164,9 @@ THREADFOO
#define NAMELEN 16
-#define errExitEN(en, msg) \e
- do { errno = en; perror(msg); exit(EXIT_FAILURE); \e
+#define errExitEN(en, msg) do \e
+ { \
+ errno = en; perror(msg); exit(EXIT_FAILURE); \e
} while (0)
static void *
--
2.28.0
Member 'tv_nsec' of 'struct timespec' is of type 'long' (see time.h.0p),
and therefore, the cast is completely redundant.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/clock_getcpuclockid.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man3/clock_getcpuclockid.3 b/man3/clock_getcpuclockid.3
index 283f92557..050d7ad13 100644
--- a/man3/clock_getcpuclockid.3
+++ b/man3/clock_getcpuclockid.3
@@ -154,7 +154,7 @@ main(int argc, char *argv[])
}
printf("CPU-time clock for PID %s is %ld.%09ld seconds\en",
- argv[1], (long) ts.tv_sec, (long) ts.tv_nsec);
+ argv[1], (long) ts.tv_sec, ts.tv_nsec);
exit(EXIT_SUCCESS);
}
.EE
--
2.28.0
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man4/loop.4 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man4/loop.4 b/man4/loop.4
> index 73b23871d..1b5d05666 100644
> --- a/man4/loop.4
> +++ b/man4/loop.4
> @@ -227,7 +227,7 @@ in
> .IR loop_config.info.lo_flags ;
> and
> .IP *
> -Explicitly request read-only mode by setting
> +explicitly request read-only mode by setting
> .BR LO_FLAGS_READ_ONLY
> in
> .IR loop_config.info.lo_flags .
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man7/aio.7 | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/man7/aio.7 b/man7/aio.7
> index ff099885e..9b2c44c48 100644
> --- a/man7/aio.7
> +++ b/man7/aio.7
> @@ -257,8 +257,6 @@ aio_return():
>
> #define errExit(msg) do { perror(msg); exit(EXIT_FAILURE); } while (0)
>
> -#define errMsg(msg) do { perror(msg); } while (0)
> -
> struct ioRequest { /* Application\-defined structure for tracking
> I/O requests */
> int reqNum;
> @@ -390,7 +388,7 @@ main(int argc, char *argv[])
> else if (s == AIO_ALLDONE)
> printf("I/O all done\en");
> else
> - errMsg("aio_cancel");
> + perror("aio_cancel");
> }
> }
>
> @@ -418,7 +416,7 @@ main(int argc, char *argv[])
> printf("Canceled\en");
> break;
> default:
> - errMsg("aio_error");
> + perror("aio_error");
> break;
> }
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> For consistency.
>
> Most man pages use 'long' instead of 'long int'.
>
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man3/get_phys_pages.3 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man3/get_phys_pages.3 b/man3/get_phys_pages.3
> index 4a9177dfd..97ba625b7 100644
> --- a/man3/get_phys_pages.3
> +++ b/man3/get_phys_pages.3
> @@ -30,8 +30,8 @@ page counts
> .nf
> .B "#include <sys/sysinfo.h>"
> .PP
> -.B long int get_phys_pages(void);
> -.B long int get_avphys_pages(void);
> +.B long get_phys_pages(void);
> +.B long get_avphys_pages(void);
> .fi
> .SH DESCRIPTION
> The function
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Notes: I copied .nf and .fi from futex.2, but they made no visual difference.
> What do they actually do?
>
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man2/membarrier.2 | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/man2/membarrier.2 b/man2/membarrier.2
> index 8825de71e..f65c6be5c 100644
> --- a/man2/membarrier.2
> +++ b/man2/membarrier.2
> @@ -26,9 +26,15 @@
> .SH NAME
> membarrier \- issue memory barriers on a set of threads
> .SH SYNOPSIS
> +.nf
> +.PP
> .B #include <linux/membarrier.h>
> .PP
> .BI "int membarrier(int " cmd ", int " flags ");"
> +.fi
> +.PP
> +.IR Note :
> +There is no glibc wrapper for this system call; see NOTES.
> .SH DESCRIPTION
> The
> .BR membarrier ()
> @@ -270,6 +276,9 @@ Examples where
> .BR membarrier ()
> can be useful include implementations
> of Read-Copy-Update libraries and garbage collectors.
> +.PP
> +Glibc does not provide a wrapper for this system call; call it using
> +.BR syscall (2).
> .SH EXAMPLES
> Assuming a multithreaded application where "fast_path()" is executed
> very frequently, and where "slow_path()" is executed infrequently, the
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man3/getpwent_r.3 | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/man3/getpwent_r.3 b/man3/getpwent_r.3
> index 0f7581091..b6c1c281f 100644
> --- a/man3/getpwent_r.3
> +++ b/man3/getpwent_r.3
> @@ -184,7 +184,8 @@ in the stream with all other threads.
> int
> main(void)
> {
> - struct passwd pw, *pwp;
> + struct passwd pw;
> + struct passwd *pwp;
> char buf[BUFLEN];
> int i;
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man3/getpwent_r.3 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man3/getpwent_r.3 b/man3/getpwent_r.3
> index 64f27dbfb..0f7581091 100644
> --- a/man3/getpwent_r.3
> +++ b/man3/getpwent_r.3
> @@ -190,7 +190,7 @@ main(void)
>
> setpwent();
> while (1) {
> - i = getpwent_r(&pw, buf, BUFLEN, &pwp);
> + i = getpwent_r(&pw, buf, sizeof(buf), &pwp);
> if (i)
> break;
> printf("%s (%d)\etHOME %s\etSHELL %s\en", pwp\->pw_name,
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch applied.
Cheers,
Michael
> ---
> man3/getgrent_r.3 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
> index 81d81a851..76deec370 100644
> --- a/man3/getgrent_r.3
> +++ b/man3/getgrent_r.3
> @@ -186,7 +186,7 @@ main(void)
>
> setgrent();
> while (1) {
> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
> if (i)
> break;
> printf("%s (%d):", grpp\->gr_name, grpp\->gr_gid);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Both major(3) and minor(3) return an 'unsigned int',
> so there is no need to use a 'long' for printing.
> Moreover, it should have been 'unsigned long',
> as "%lx" expects an unsigned type.
This may be true on Linux, but is not true on other systems.
For example, on HP-UX, according to one header file I'm
looking at, the return value is 'long'.
These kinds of casts are intended to improve code portability
across UNIX implementations, so I think they should stay
(although, I do wonder if they would be even better as casts
to 'unsigned long')
Thanks,
Michael
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man2/ioctl_ns.2 | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/man2/ioctl_ns.2 b/man2/ioctl_ns.2
> index 818dde32c..bf832a2c7 100644
> --- a/man2/ioctl_ns.2
> +++ b/man2/ioctl_ns.2
> @@ -316,9 +316,8 @@ main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
> printf("Device/Inode of owning user namespace is: "
> - "[%lx,%lx] / %ld\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev),
> - (long) sb.st_ino);
> + "[%x,%x] / %ld\en",
> + major(sb.st_dev), minor(sb.st_dev), (long) sb.st_ino);
>
> close(userns_fd);
> }
> @@ -345,9 +344,8 @@ main(int argc, char *argv[])
> perror("fstat\-parentns");
> exit(EXIT_FAILURE);
> }
> - printf("Device/Inode of parent namespace is: [%lx,%lx] / %ld\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev),
> - (long) sb.st_ino);
> + printf("Device/Inode of parent namespace is: [%x,%x] / %ld\en",
> + major(sb.st_dev), minor(sb.st_dev), (long) sb.st_ino);
>
> close(parent_fd);
> }
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Both major(3) and minor(3) return an 'unsigned int',
> so there is no need to use a 'long' for printing.
> Moreover, it should have been 'unsigned long',
> as "%lx" expects an unsigned type.
See my reply to patch 10/24.
Thanks,
Michael
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man2/stat.2 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man2/stat.2 b/man2/stat.2
> index b35e3c615..3418e1b5d 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -664,8 +664,8 @@ main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
>
> - printf("ID of containing device: [%lx,%lx]\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev));
> + printf("ID of containing device: [%x,%x]\en",
> + major(sb.st_dev), minor(sb.st_dev));
>
> printf("File type: ");
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Member 'tv_nsec' of 'struct timespec' is of type 'long' (see time.h.0p),
> and therefore, the cast is completely redundant.
Good catch! Patch applied.
Cheers,
Michael
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man3/clock_getcpuclockid.3 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man3/clock_getcpuclockid.3 b/man3/clock_getcpuclockid.3
> index 283f92557..050d7ad13 100644
> --- a/man3/clock_getcpuclockid.3
> +++ b/man3/clock_getcpuclockid.3
> @@ -154,7 +154,7 @@ main(int argc, char *argv[])
> }
>
> printf("CPU-time clock for PID %s is %ld.%09ld seconds\en",
> - argv[1], (long) ts.tv_sec, (long) ts.tv_nsec);
> + argv[1], (long) ts.tv_sec, ts.tv_nsec);
> exit(EXIT_SUCCESS);
> }
> .EE
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man2/add_key.2 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/add_key.2 b/man2/add_key.2
> index bda08c3d2..bbd2d1d03 100644
> --- a/man2/add_key.2
> +++ b/man2/add_key.2
> @@ -262,7 +262,7 @@ main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
>
> - printf("Key ID is %lx\en", (long) key);
> + printf("Key ID is %lx\en", (unsigned long) key);
>
> exit(EXIT_SUCCESS);
> }
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man2/request_key.2 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/request_key.2 b/man2/request_key.2
> index e28c11ded..c14bcb000 100644
> --- a/man2/request_key.2
> +++ b/man2/request_key.2
> @@ -537,7 +537,7 @@ main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
>
> - printf("Key ID is %lx\en", (long) key);
> + printf("Key ID is %lx\en", (unsigned long) key);
>
> exit(EXIT_SUCCESS);
> }
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
I'm reluctant to apply this, because MAX() is not a standard
macro. I suppose it may not be present on some other UNIX
systems. You thoughts?
Cheers,
Michael
> ---
> man2/select_tut.2 | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/man2/select_tut.2 b/man2/select_tut.2
> index f683dd49d..d23683d75 100644
> --- a/man2/select_tut.2
> +++ b/man2/select_tut.2
> @@ -354,6 +354,7 @@ from one TCP port to another.
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> +#include <sys/param.h> /* Definition of MAX(a, b) */
> #include <sys/select.h>
> #include <string.h>
> #include <signal.h>
> @@ -364,9 +365,6 @@ from one TCP port to another.
>
> static int forward_port;
>
> -#undef max
> -#define max(x,y) ((x) > (y) ? (x) : (y))
> -
> static int
> listen_socket(int listen_port)
> {
> @@ -483,7 +481,7 @@ main(int argc, char *argv[])
> FD_ZERO(&writefds);
> FD_ZERO(&exceptfds);
> FD_SET(h, &readfds);
> - nfds = max(nfds, h);
> + nfds = MAX(nfds, h);
>
> if (fd1 > 0 && buf1_avail < BUF_SIZE)
> FD_SET(fd1, &readfds);
> @@ -499,11 +497,11 @@ main(int argc, char *argv[])
>
> if (fd1 > 0) {
> FD_SET(fd1, &exceptfds);
> - nfds = max(nfds, fd1);
> + nfds = MAX(nfds, fd1);
> }
> if (fd2 > 0) {
> FD_SET(fd2, &exceptfds);
> - nfds = max(nfds, fd2);
> + nfds = MAX(nfds, fd2);
> }
>
> ready = select(nfds + 1, &readfds, &writefds, &exceptfds, NULL);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man2/timer_create.2 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man2/timer_create.2 b/man2/timer_create.2
> index e9a8b8503..9c9907739 100644
> --- a/man2/timer_create.2
> +++ b/man2/timer_create.2
> @@ -390,7 +390,7 @@ print_siginfo(siginfo_t *si)
> tidp = si\->si_value.sival_ptr;
>
> printf(" sival_ptr = %p; ", si\->si_value.sival_ptr);
> - printf(" *sival_ptr = 0x%lx\en", (long) *tidp);
> + printf(" *sival_ptr = 0x%lx\en", (unsigned long) *tidp);
>
> or = timer_getoverrun(*tidp);
> if (or == \-1)
> @@ -454,7 +454,7 @@ main(int argc, char *argv[])
> if (timer_create(CLOCKID, &sev, &timerid) == \-1)
> errExit("timer_create");
>
> - printf("timer ID is 0x%lx\en", (long) timerid);
> + printf("timer ID is 0x%lx\en", (unsigned long) timerid);
>
> /* Start the timer */
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man3/pthread_setname_np.3 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
What's the rationale for this patch?
Thanks,
Michael
>
> diff --git a/man3/pthread_setname_np.3 b/man3/pthread_setname_np.3
> index b206f66c0..4dc4960cd 100644
> --- a/man3/pthread_setname_np.3
> +++ b/man3/pthread_setname_np.3
> @@ -164,8 +164,9 @@ THREADFOO
>
> #define NAMELEN 16
>
> -#define errExitEN(en, msg) \e
> - do { errno = en; perror(msg); exit(EXIT_FAILURE); \e
> +#define errExitEN(en, msg) do \e
> + { \
> + errno = en; perror(msg); exit(EXIT_FAILURE); \e
> } while (0)
>
> static void *
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
I don't have strong feelings for or against this patch. I guess
it does make the code more expressive, which is good. Patch applied.
Thanks,
Michael
> ---
> man3/fread.3 | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/man3/fread.3 b/man3/fread.3
> index 4c5dc3dbc..8e71e620e 100644
> --- a/man3/fread.3
> +++ b/man3/fread.3
> @@ -136,6 +136,8 @@ Class: 0x02
> #include <stdio.h>
> #include <stdlib.h>
>
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> +
> int
> main(void)
> {
> @@ -147,9 +149,7 @@ main(void)
>
> unsigned char buffer[4];
>
> - size_t ret =
> - fread(buffer, sizeof(buffer) / sizeof(*buffer), sizeof(*buffer),
> - fp);
> + size_t ret = fread(buffer, ARRAY_SIZE(buffer), sizeof(*buffer), fp);
> if (ret != sizeof(*buffer)) {
> fprintf(stderr, "fread() failed: %zu\en", ret);
> exit(EXIT_FAILURE);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man7/unix.7 | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/man7/unix.7 b/man7/unix.7
> index f61b51424..50828a5bc 100644
> --- a/man7/unix.7
> +++ b/man7/unix.7
> @@ -991,7 +991,7 @@ main(int argc, char *argv[])
>
> /* Wait for next data packet. */
>
> - ret = read(data_socket, buffer, BUFFER_SIZE);
> + ret = read(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("read");
> exit(EXIT_FAILURE);
> @@ -999,16 +999,16 @@ main(int argc, char *argv[])
>
> /* Ensure buffer is 0\-terminated. */
>
> - buffer[BUFFER_SIZE \- 1] = 0;
> + buffer[sizeof(buffer) \- 1] = 0;
>
> /* Handle commands. */
>
> - if (!strncmp(buffer, "DOWN", BUFFER_SIZE)) {
> + if (!strncmp(buffer, "DOWN", sizeof(buffer))) {
> down_flag = 1;
> break;
> }
>
> - if (!strncmp(buffer, "END", BUFFER_SIZE)) {
> + if (!strncmp(buffer, "END", sizeof(buffer))) {
> break;
> }
>
> @@ -1020,7 +1020,7 @@ main(int argc, char *argv[])
> /* Send result. */
>
> sprintf(buffer, "%d", result);
> - ret = write(data_socket, buffer, BUFFER_SIZE);
> + ret = write(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("write");
> exit(EXIT_FAILURE);
> @@ -1116,7 +1116,7 @@ main(int argc, char *argv[])
>
> /* Receive result. */
>
> - ret = read(data_socket, buffer, BUFFER_SIZE);
> + ret = read(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("read");
> exit(EXIT_FAILURE);
> @@ -1124,7 +1124,7 @@ main(int argc, char *argv[])
>
> /* Ensure buffer is 0\-terminated. */
>
> - buffer[BUFFER_SIZE \- 1] = 0;
> + buffer[sizeof(buffer) \- 1] = 0;
>
> printf("Result = %s\en", buffer);
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man7/unix.7 | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/man7/unix.7 b/man7/unix.7
> index f61b51424..50828a5bc 100644
> --- a/man7/unix.7
> +++ b/man7/unix.7
> @@ -991,7 +991,7 @@ main(int argc, char *argv[])
>
> /* Wait for next data packet. */
>
> - ret = read(data_socket, buffer, BUFFER_SIZE);
> + ret = read(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("read");
> exit(EXIT_FAILURE);
> @@ -999,16 +999,16 @@ main(int argc, char *argv[])
>
> /* Ensure buffer is 0\-terminated. */
>
> - buffer[BUFFER_SIZE \- 1] = 0;
> + buffer[sizeof(buffer) \- 1] = 0;
>
> /* Handle commands. */
>
> - if (!strncmp(buffer, "DOWN", BUFFER_SIZE)) {
> + if (!strncmp(buffer, "DOWN", sizeof(buffer))) {
> down_flag = 1;
> break;
> }
>
> - if (!strncmp(buffer, "END", BUFFER_SIZE)) {
> + if (!strncmp(buffer, "END", sizeof(buffer))) {
> break;
> }
>
> @@ -1020,7 +1020,7 @@ main(int argc, char *argv[])
> /* Send result. */
>
> sprintf(buffer, "%d", result);
> - ret = write(data_socket, buffer, BUFFER_SIZE);
> + ret = write(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("write");
> exit(EXIT_FAILURE);
> @@ -1116,7 +1116,7 @@ main(int argc, char *argv[])
>
> /* Receive result. */
>
> - ret = read(data_socket, buffer, BUFFER_SIZE);
> + ret = read(data_socket, buffer, sizeof(buffer));
> if (ret == \-1) {
> perror("read");
> exit(EXIT_FAILURE);
> @@ -1124,7 +1124,7 @@ main(int argc, char *argv[])
>
> /* Ensure buffer is 0\-terminated. */
>
> - buffer[BUFFER_SIZE \- 1] = 0;
> + buffer[sizeof(buffer) \- 1] = 0;
>
> printf("Result = %s\en", buffer);
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man3/offsetof.3 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man3/offsetof.3 b/man3/offsetof.3
> index 05fdd6dd8..f0c9975f4 100644
> --- a/man3/offsetof.3
> +++ b/man3/offsetof.3
> @@ -93,10 +93,10 @@ main(void)
>
> /* Output is compiler dependent */
>
> - printf("offsets: i=%zd; c=%zd; d=%zd a=%zd\en",
> + printf("offsets: i=%zu; c=%zu; d=%zu a=%zu\en",
> offsetof(struct s, i), offsetof(struct s, c),
> offsetof(struct s, d), offsetof(struct s, a));
> - printf("sizeof(struct s)=%zd\en", sizeof(struct s));
> + printf("sizeof(struct s)=%zu\en", sizeof(struct s));
>
> exit(EXIT_SUCCESS);
> }
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> 'nread' is of type 'ssize_t'
> 'tot' adds up different values contained in 'nread',
> so it should also be 'ssize_t', and not 'int' (which possibly overflows).
>
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man5/core.5 | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/man5/core.5 b/man5/core.5
> index a87ebbaf4..45c9de845 100644
> --- a/man5/core.5
> +++ b/man5/core.5
> @@ -654,8 +654,7 @@ Total bytes in core dump: 282624
> int
> main(int argc, char *argv[])
> {
> - int tot;
> - ssize_t nread;
> + ssize_t nread, tot;
> char buf[BUF_SIZE];
> FILE *fp;
> char cwd[PATH_MAX];
> @@ -684,7 +683,7 @@ main(int argc, char *argv[])
> tot = 0;
> while ((nread = read(STDIN_FILENO, buf, BUF_SIZE)) > 0)
> tot += nread;
> - fprintf(fp, "Total bytes in core dump: %d\en", tot);
> + fprintf(fp, "Total bytes in core dump: %zd\en", tot);
>
> fclose(fp);
> exit(EXIT_SUCCESS);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man2/timerfd_create.2 | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/man2/timerfd_create.2 b/man2/timerfd_create.2
> index 67a13dba3..90e35d9b5 100644
> --- a/man2/timerfd_create.2
> +++ b/man2/timerfd_create.2
> @@ -624,6 +624,7 @@ a.out 3 1 100
> #include <sys/timerfd.h>
> #include <time.h>
> #include <unistd.h>
> +#include <inttypes.h> /* Definition of PRIu64 */
> #include <stdlib.h>
> #include <stdio.h>
> #include <stdint.h> /* Definition of uint64_t */
> @@ -706,9 +707,7 @@ main(int argc, char *argv[])
>
> tot_exp += exp;
> print_elapsed_time();
> - printf("read: %llu; total=%llu\en",
> - (unsigned long long) exp,
> - (unsigned long long) tot_exp);
> + printf("read: %"PRIu64"; total=%"PRIu64"\en", exp, tot_exp);
> }
>
> exit(EXIT_SUCCESS);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Thanks, Alex. Patch Applied.
Cheers,
Michael
> ---
> man2/eventfd.2 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man2/eventfd.2 b/man2/eventfd.2
> index 929234ab7..71e9d85b4 100644
> --- a/man2/eventfd.2
> +++ b/man2/eventfd.2
> @@ -386,6 +386,7 @@ Parent read 28 (0x1c) from efd
> .EX
> #include <sys/eventfd.h>
> #include <unistd.h>
> +#include <inttypes.h> /* Definition of PRIu64 & PRIx64 */
> #include <stdlib.h>
> #include <stdio.h>
> #include <stdint.h> /* Definition of uint64_t */
> @@ -430,8 +431,7 @@ main(int argc, char *argv[])
> s = read(efd, &u, sizeof(uint64_t));
> if (s != sizeof(uint64_t))
> handle_error("read");
> - printf("Parent read %llu (0x%llx) from efd\en",
> - (unsigned long long) u, (unsigned long long) u);
> + printf("Parent read %"PRIu64" (0x%"PRIx64") from efd\en", u, u);
> exit(EXIT_SUCCESS);
>
> case \-1:
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Michael,
The indentation in the original code was a bit weird (specifically, the
'do {' part had one more indentation level than the closing '} while'),
so I simply chose something nice. See the original page, and if you
think it's ok keep it, else find something nice :)
Cheers,
Alex
On 2020-09-11 09:58, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Signed-off-by: Alejandro Colomar <[email protected]>
>> ---
>> man3/pthread_setname_np.3 | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> What's the rationale for this patch?
>
> Thanks,
>
> Michael
>>
>> diff --git a/man3/pthread_setname_np.3 b/man3/pthread_setname_np.3
>> index b206f66c0..4dc4960cd 100644
>> --- a/man3/pthread_setname_np.3
>> +++ b/man3/pthread_setname_np.3
>> @@ -164,8 +164,9 @@ THREADFOO
>>
>> #define NAMELEN 16
>>
>> -#define errExitEN(en, msg) \e
>> - do { errno = en; perror(msg); exit(EXIT_FAILURE); \e
>> +#define errExitEN(en, msg) do \e
>> + { \
>> + errno = en; perror(msg); exit(EXIT_FAILURE); \e
>> } while (0)
>>
>> static void *
>>
>
>
Hi Michael,
On 2020-09-11 09:54, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Signed-off-by: Alejandro Colomar <[email protected]>
>
> I'm reluctant to apply this, because MAX() is not a standard
> macro. I suppose it may not be present on some other UNIX
> systems. You thoughts?
I know the BSDs have it; maybe not all of them (I don't know them all),
but it is present at least in OpenBSD, libbsd, FreeBSD so I guess it's
common enough.
For other UNIX systems, I have no idea.
Maybe there's some unicorn UNIX that doesn't have it... impossible to tell.
Cheers,
Alex
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> I added some headers to reduce the number of warnings.
> I found the needed headers by using grep, but maybe some of them
> shouldn't be included directly.
>
> The example still has many problems to compile.
Yes, there are so many problems there, I'm not sure it's really worth
adding the header files. It increases the impression that this is
somehow a complete program, when it's not. I agree this is a bit of
a mess, but I think it's probably best to leave the example as is.
As the manual page says:
Some complete working code can be found in the samples/bpf direc‐
tory in the kernel source tree.
Thanks,
Michael
>
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man2/bpf.2 | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/man2/bpf.2 b/man2/bpf.2
> index b45acde76..d26d6a43d 100644
> --- a/man2/bpf.2
> +++ b/man2/bpf.2
> @@ -981,6 +981,18 @@ ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> * 3. attach prog_fd to raw socket via setsockopt()
> * 4. print number of received TCP/UDP packets every second
> */
> +#include <assert.h>
> +#include <errno.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +#include <unistd.h>
> +#include <linux/bpf.h>
> +#include <linux/if_ether.h>
> +#include <linux/in.h>
> +#include <linux/ip.h>
> +
> int
> main(int argc, char **argv)
> {
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Michael,
On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
> This may be true on Linux, but is not true on other systems.
> For example, on HP-UX, according to one header file I'm
> looking at, the return value is 'long'. >
> These kinds of casts are intended to improve code portability
> across UNIX implementations, so I think they should stay
> (although, I do wonder if they would be even better as casts
> to 'unsigned long')
Fine, then here is the patch with the casts to 'unsigned long'.
Cheers,
Alex
-------------------------------------------------------------
From c5f644e798ffc5dec0c73f324a26059568865c68 Mon Sep 17 00:00:00 2001
From: Alejandro Colomar <[email protected]>
Date: Fri, 11 Sep 2020 10:51:26 +0200
Subject: [PATCH v2 10/24] ioctl_ns.2: Cast to 'unsigned long' rather
than 'long'
when printing with "%lx"
From the email conversation:
On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Both major(3) and minor(3) return an 'unsigned int',
>> so there is no need to use a 'long' for printing.
>> Moreover, it should have been 'unsigned long',
>> as "%lx" expects an unsigned type.
>
> This may be true on Linux, but is not true on other systems.
> For example, on HP-UX, according to one header file I'm
> looking at, the return value is 'long'.
>
> These kinds of casts are intended to improve code portability
> across UNIX implementations, so I think they should stay
> (although, I do wonder if they would be even better as casts
> to 'unsigned long')
>
> Thanks,
>
> Michael
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/ioctl_ns.2 | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/man2/ioctl_ns.2 b/man2/ioctl_ns.2
index 818dde32c..8b8789d1f 100644
--- a/man2/ioctl_ns.2
+++ b/man2/ioctl_ns.2
@@ -317,7 +317,8 @@ main(int argc, char *argv[])
}
printf("Device/Inode of owning user namespace is: "
"[%lx,%lx] / %ld\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev),
+ (unsigned long) major(sb.st_dev),
+ (unsigned long) minor(sb.st_dev),
(long) sb.st_ino);
close(userns_fd);
@@ -346,7 +347,8 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
printf("Device/Inode of parent namespace is: [%lx,%lx] / %ld\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev),
+ (unsigned long) major(sb.st_dev),
+ (unsigned long) minor(sb.st_dev),
(long) sb.st_ino);
close(parent_fd);
--
2.28.0
Hi Michael,
On 2020-09-11 09:25, Michael Kerrisk (man-pages) wrote:
> See my reply to patch 10/24.
As with 10/24, here's the new version.
Cheers,
Alex
--------------------------------------------------------
From 911c791f0168851cdfdb30a65b6935011e4a161c Mon Sep 17 00:00:00 2001
From: Alejandro Colomar <[email protected]>
Date: Fri, 11 Sep 2020 10:52:14 +0200
Subject: [PATCH v2 11/24] stat.2: Cast to 'unsigned long' rather than 'long'
when printing with "%lx"
From the email conversation:
On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Both major(3) and minor(3) return an 'unsigned int',
>> so there is no need to use a 'long' for printing.
>> Moreover, it should have been 'unsigned long',
>> as "%lx" expects an unsigned type.
>
> This may be true on Linux, but is not true on other systems.
> For example, on HP-UX, according to one header file I'm
> looking at, the return value is 'long'.
>
> These kinds of casts are intended to improve code portability
> across UNIX implementations, so I think they should stay
> (although, I do wonder if they would be even better as casts
> to 'unsigned long')
>
> Thanks,
>
> Michael
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/stat.2 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/man2/stat.2 b/man2/stat.2
index b35e3c615..08af24c56 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -665,7 +665,8 @@ main(int argc, char *argv[])
}
printf("ID of containing device: [%lx,%lx]\en",
- (long) major(sb.st_dev), (long) minor(sb.st_dev));
+ (unsigned long) major(sb.st_dev),
+ (unsigned long) minor(sb.st_dev));
printf("File type: ");
--
2.28.0
On 9/11/20 11:13 AM, Alejandro Colomar wrote:
> Hi Michael,
>
> On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
>> This may be true on Linux, but is not true on other systems.
>> For example, on HP-UX, according to one header file I'm
>> looking at, the return value is 'long'. >
>> These kinds of casts are intended to improve code portability
>> across UNIX implementations, so I think they should stay
>> (although, I do wonder if they would be even better as casts
>> to 'unsigned long')
>
> Fine, then here is the patch with the casts to 'unsigned long'.
Thanks. Patch applied.
Cheers,
Michael
> -------------------------------------------------------------
> From c5f644e798ffc5dec0c73f324a26059568865c68 Mon Sep 17 00:00:00 2001
> From: Alejandro Colomar <[email protected]>
> Date: Fri, 11 Sep 2020 10:51:26 +0200
> Subject: [PATCH v2 10/24] ioctl_ns.2: Cast to 'unsigned long' rather
> than 'long'
> when printing with "%lx"
>
> From the email conversation:
>
> On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
> > Hi Alex,
> >
> > On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> >> Both major(3) and minor(3) return an 'unsigned int',
> >> so there is no need to use a 'long' for printing.
> >> Moreover, it should have been 'unsigned long',
> >> as "%lx" expects an unsigned type.
> >
> > This may be true on Linux, but is not true on other systems.
> > For example, on HP-UX, according to one header file I'm
> > looking at, the return value is 'long'.
> >
> > These kinds of casts are intended to improve code portability
> > across UNIX implementations, so I think they should stay
> > (although, I do wonder if they would be even better as casts
> > to 'unsigned long')
> >
> > Thanks,
> >
> > Michael
>
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man2/ioctl_ns.2 | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/man2/ioctl_ns.2 b/man2/ioctl_ns.2
> index 818dde32c..8b8789d1f 100644
> --- a/man2/ioctl_ns.2
> +++ b/man2/ioctl_ns.2
> @@ -317,7 +317,8 @@ main(int argc, char *argv[])
> }
> printf("Device/Inode of owning user namespace is: "
> "[%lx,%lx] / %ld\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev),
> + (unsigned long) major(sb.st_dev),
> + (unsigned long) minor(sb.st_dev),
> (long) sb.st_ino);
>
> close(userns_fd);
> @@ -346,7 +347,8 @@ main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
> printf("Device/Inode of parent namespace is: [%lx,%lx] / %ld\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev),
> + (unsigned long) major(sb.st_dev),
> + (unsigned long) minor(sb.st_dev),
> (long) sb.st_ino);
>
> close(parent_fd);
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/11/20 11:16 AM, Alejandro Colomar wrote:
> Hi Michael,
>
> On 2020-09-11 09:25, Michael Kerrisk (man-pages) wrote:
>> See my reply to patch 10/24.
>
> As with 10/24, here's the new version.
Thanks, Alex. Applied.
Cheers,
Michael
> --------------------------------------------------------
> From 911c791f0168851cdfdb30a65b6935011e4a161c Mon Sep 17 00:00:00 2001
> From: Alejandro Colomar <[email protected]>
> Date: Fri, 11 Sep 2020 10:52:14 +0200
> Subject: [PATCH v2 11/24] stat.2: Cast to 'unsigned long' rather than 'long'
> when printing with "%lx"
>
> From the email conversation:
>
> On 2020-09-11 09:24, Michael Kerrisk (man-pages) wrote:
> > Hi Alex,
> >
> > On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> >> Both major(3) and minor(3) return an 'unsigned int',
> >> so there is no need to use a 'long' for printing.
> >> Moreover, it should have been 'unsigned long',
> >> as "%lx" expects an unsigned type.
> >
> > This may be true on Linux, but is not true on other systems.
> > For example, on HP-UX, according to one header file I'm
> > looking at, the return value is 'long'.
> >
> > These kinds of casts are intended to improve code portability
> > across UNIX implementations, so I think they should stay
> > (although, I do wonder if they would be even better as casts
> > to 'unsigned long')
> >
> > Thanks,
> >
> > Michael
>
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man2/stat.2 | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/man2/stat.2 b/man2/stat.2
> index b35e3c615..08af24c56 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -665,7 +665,8 @@ main(int argc, char *argv[])
> }
>
> printf("ID of containing device: [%lx,%lx]\en",
> - (long) major(sb.st_dev), (long) minor(sb.st_dev));
> + (unsigned long) major(sb.st_dev),
> + (unsigned long) minor(sb.st_dev));
>
> printf("File type: ");
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Hi Michael,
>
> I have a lot of patches here.
> Some of them are trivial, and some of them are not.
> I have them sorted by their contents more or less,
Yes, thanks for the sorting!
> but if you
> prefer completely unrelated email threads for completely unrelated
> patches, please tell me.
The sorting was sufficient for me. Thanks for all of the patches!
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Alex,
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man3/inet_net_pton.3 | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/man3/inet_net_pton.3 b/man3/inet_net_pton.3
> index 00f94b9d4..d74a33d74 100644
> --- a/man3/inet_net_pton.3
> +++ b/man3/inet_net_pton.3
> @@ -332,6 +332,7 @@ Raw address: c1a80180
> /* Link with "\-lresolv" */
>
> #include <arpa/inet.h>
> +#include <inttypes.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> @@ -381,7 +382,7 @@ main(int argc, char *argv[])
> may not have been touched by inet_net_ntop(), and so will still
> have any initial value that was specified in argv[2]. */
>
> - printf("Raw address: %x\en", htonl(addr.s_addr));
> + printf("Raw address: %"PRIx32"\en", htonl(addr.s_addr));
>
> exit(EXIT_SUCCESS);
> }
So, I'm in a little bit of doubt about patches 01 and 02. Does
this really win us anything? On the one hand, %"PRIx32" is more
difficult to read than %x. On the other, does it win us anything
in terms of portability? At first glance, the answers seems to me
to be "no". Your thoughts?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Michael,
On 2020-09-11 11:19, Michael Kerrisk (man-pages) wrote:
> On 9/11/20 11:16 AM, Alejandro Colomar wrote:
>> Hi Michael,
>>
>> On 2020-09-11 09:25, Michael Kerrisk (man-pages) wrote:
>>> See my reply to patch 10/24.
>>
>> As with 10/24, here's the new version.
>
> Thanks, Alex. Applied.
>
> Cheers,
>
> Michael
I accidentally used a tab instead of spaces in my last commit.
This fixes that by using only spaces.
Please apply this patch on top of (already applied) v2 11/24.
Thanks,
Alex
----------------------------------------------------------------------
From daad53ccc36a75a0a9928e0807de38925b9b1433 Mon Sep 17 00:00:00 2001
From: Alejandro Colomar <[email protected]>
Date: Fri, 11 Sep 2020 11:29:40 +0200
Subject: [PATCH v3 11/24] stat.2: wsfix
I accidentally used a tab instead of spaces in my last commit.
This fixes that by using only spaces.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/stat.2 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man2/stat.2 b/man2/stat.2
index 08af24c56..7e5417480 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -665,7 +665,7 @@ main(int argc, char *argv[])
}
printf("ID of containing device: [%lx,%lx]\en",
- (unsigned long) major(sb.st_dev),
+ (unsigned long) major(sb.st_dev),
(unsigned long) minor(sb.st_dev));
printf("File type: ");
--
2.28.0
Hi Michael,
On 2020-09-11 11:31, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Signed-off-by: Alejandro Colomar <[email protected]>
>> ---
>> man3/inet_net_pton.3 | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/man3/inet_net_pton.3 b/man3/inet_net_pton.3
>> index 00f94b9d4..d74a33d74 100644
>> --- a/man3/inet_net_pton.3
>> +++ b/man3/inet_net_pton.3
>> @@ -332,6 +332,7 @@ Raw address: c1a80180
>> /* Link with "\-lresolv" */
>>
>> #include <arpa/inet.h>
>> +#include <inttypes.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>>
>> @@ -381,7 +382,7 @@ main(int argc, char *argv[])
>> may not have been touched by inet_net_ntop(), and so will still
>> have any initial value that was specified in argv[2]. */
>>
>> - printf("Raw address: %x\en", htonl(addr.s_addr));
>> + printf("Raw address: %"PRIx32"\en", htonl(addr.s_addr));
>>
>> exit(EXIT_SUCCESS);
>> }
>
> So, I'm in a little bit of doubt about patches 01 and 02. Does
> this really win us anything? On the one hand, %"PRIx32" is more
> difficult to read than %x. On the other, does it win us anything
> in terms of portability? At first glance, the answers seems to me
> to be "no". Your thoughts?
>
> Thanks,
>
> Michael
On 16-bit systems 'unsigned int' might be shorter than 'uint32_t'.
There it would make a difference, I guess.
Thanks,
Alex
Hi Alex,
On 9/11/20 11:39 AM, Alejandro Colomar wrote:
> Hi Michael,
[...]
>> So, I'm in a little bit of doubt about patches 01 and 02. Does
>> this really win us anything? On the one hand, %"PRIx32" is more
>> difficult to read than %x. On the other, does it win us anything
>> in terms of portability? At first glance, the answers seems to me
>> to be "no". Your thoughts?
>>
>> Thanks,
>>
>> Michael
>
> On 16-bit systems 'unsigned int' might be shorter than 'uint32_t'.
> There it would make a difference, I guess.
I guess. But even un the 80s, when (I think) htonl() appeared,
I kind of doubt that there were many such systems. I think
I'll skip these patches.
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/11/20 10:46 AM, Alejandro Colomar wrote:
> Hi Michael,
>
> On 2020-09-11 09:54, Michael Kerrisk (man-pages) wrote:
>> Hi Alex,
>>
>> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>>> Signed-off-by: Alejandro Colomar <[email protected]>
>>
>> I'm reluctant to apply this, because MAX() is not a standard
>> macro. I suppose it may not be present on some other UNIX
>> systems. You thoughts?
>
> I know the BSDs have it; maybe not all of them (I don't know them all),
> but it is present at least in OpenBSD, libbsd, FreeBSD so I guess it's
> common enough.
> For other UNIX systems, I have no idea.
> Maybe there's some unicorn UNIX that doesn't have it... impossible to tell.
Yup. So, I think I will skip this patch.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <[email protected]>
Just noting here for completeness: see the discussion in patch 01.
Thanks,
Michael
> ---
> man3/endian.3 | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/man3/endian.3 b/man3/endian.3
> index bdf1efd7e..3a898bb02 100644
> --- a/man3/endian.3
> +++ b/man3/endian.3
> @@ -147,6 +147,7 @@ htobe32(x.u32) = 0x11223344
> \&
> .EX
> #include <endian.h>
> +#include <inttypes.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -164,9 +165,9 @@ main(int argc, char *argv[])
> x.arr[2] = 0x33;
> x.arr[3] = 0x44; /* Highest-address byte */
>
> - printf("x.u32 = 0x%x\en", x.u32);
> - printf("htole32(x.u32) = 0x%x\en", htole32(x.u32));
> - printf("htobe32(x.u32) = 0x%x\en", htobe32(x.u32));
> + printf("x.u32 = 0x%"PRIx32"\en", x.u32);
> + printf("htole32(x.u32) = 0x%"PRIx32"\en", htole32(x.u32));
> + printf("htobe32(x.u32) = 0x%"PRIx32"\en", htobe32(x.u32));
>
> exit(EXIT_SUCCESS);
> }
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Michael,
On 2020-09-11 09:48, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>> Member 'tv_nsec' of 'struct timespec' is of type 'long' (see time.h.0p),
>> and therefore, the cast is completely redundant.
>
> Good catch! Patch applied.
>
> Cheers,
>
> Michael
Ping :)
Did you really apply it? hehe
Cheers,
Alex
>
>> Signed-off-by: Alejandro Colomar <[email protected]>
>> ---
>> man3/clock_getcpuclockid.3 | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/man3/clock_getcpuclockid.3 b/man3/clock_getcpuclockid.3
>> index 283f92557..050d7ad13 100644
>> --- a/man3/clock_getcpuclockid.3
>> +++ b/man3/clock_getcpuclockid.3
>> @@ -154,7 +154,7 @@ main(int argc, char *argv[])
>> }
>>
>> printf("CPU-time clock for PID %s is %ld.%09ld seconds\en",
>> - argv[1], (long) ts.tv_sec, (long) ts.tv_nsec);
>> + argv[1], (long) ts.tv_sec, ts.tv_nsec);
>> exit(EXIT_SUCCESS);
>> }
>> .EE
>>
>
>
On 9/11/20 12:25 PM, Alejandro Colomar wrote:
> Hi Michael,
>
> On 2020-09-11 09:48, Michael Kerrisk (man-pages) wrote:
>> Hi Alex,
>>
>> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>>> Member 'tv_nsec' of 'struct timespec' is of type 'long' (see time.h.0p),
>>> and therefore, the cast is completely redundant.
>>
>> Good catch! Patch applied.
>>
>> Cheers,
>>
>> Michael
>
> Ping :)
> Did you really apply it? hehe
D'oh! Now I did.
Thanks,
Michael
Hi Alex,
On 9/11/20 10:32 AM, Alejandro Colomar wrote:
> Hi Michael,
>
> The indentation in the original code was a bit weird (specifically, the
> 'do {' part had one more indentation level than the closing '} while'),
> so I simply chose something nice. See the original page, and if you
> think it's ok keep it, else find something nice :)
Sorry -- I was being a bit slow. Now I see what you mean.
I've fixed it, but in a different way.
Thanks,
Michael
> On 2020-09-11 09:58, Michael Kerrisk (man-pages) wrote:
>> Hi Alex,
>>
>> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
>>> Signed-off-by: Alejandro Colomar <[email protected]>
>>> ---
>>> man3/pthread_setname_np.3 | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> What's the rationale for this patch?
>>
>> Thanks,
>>
>> Michael
>>>
>>> diff --git a/man3/pthread_setname_np.3 b/man3/pthread_setname_np.3
>>> index b206f66c0..4dc4960cd 100644
>>> --- a/man3/pthread_setname_np.3
>>> +++ b/man3/pthread_setname_np.3
>>> @@ -164,8 +164,9 @@ THREADFOO
>>>
>>> #define NAMELEN 16
>>>
>>> -#define errExitEN(en, msg) \e
>>> - do { errno = en; perror(msg); exit(EXIT_FAILURE); \e
>>> +#define errExitEN(en, msg) do \e
>>> + { \
>>> + errno = en; perror(msg); exit(EXIT_FAILURE); \e
>>> } while (0)
>>>
>>> static void *
>>>
>>
>>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
BUFLEN should be remove completely or stay
jm2c
wh
________________________________________
Von: [email protected] [[email protected]] im Auftrag von Alejandro Colomar [[email protected]]
Gesendet: Donnerstag, 10. September 2020 23:13
An: [email protected]
Cc: [email protected]; [email protected]; Alejandro Colomar
Betreff: [PATCH 12/24] getgrent_r.3: Use sizeof() to get buffer size (instead of hardcoding macro name)
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/getgrent_r.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
index 81d81a851..76deec370 100644
--- a/man3/getgrent_r.3
+++ b/man3/getgrent_r.3
@@ -186,7 +186,7 @@ main(void)
setgrent();
while (1) {
- i = getgrent_r(&grp, buf, BUFLEN, &grpp);
+ i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
if (i)
break;
printf("%s (%d):", grpp\->gr_name, grpp\->gr_gid);
--
2.28.0
Hi Stefan,
On 2020-09-11 16:35, Stefan Puiu wrote:
> Hi,
>
> On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
> <[email protected]> wrote:
>>
>> Signed-off-by: Alejandro Colomar <[email protected]>
>> ---
>> man3/getgrent_r.3 | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
>> index 81d81a851..76deec370 100644
>> --- a/man3/getgrent_r.3
>> +++ b/man3/getgrent_r.3
>> @@ -186,7 +186,7 @@ main(void)
>>
>> setgrent();
>> while (1) {
>> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
>> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
>
> I'm worried that less attentive people might copy/paste parts of this
> in their code, where maybe buf is just a pointer, and expect it to
> work. Maybe leaving BUFLEN here is useful as a reminder that they need
> to change something to adapt the code?
>
> Just my 2 cents,
> Stefan.
>
That's a very good point.
So we have 3 options and I will propose now a 4th one. Let's see all
of them and see which one is better for the man pages.
1.- Use the macro everywhere.
pros:
- It is still valid when the buffer is a pointer and not an array.
cons:
- Hardcodes the initializer. If the array is later initialized with a
different value, it may produce a silent bug, or a compilation break.
2.- Use sizeof() everywhere, and the macro for the initializer.
pros:
- It is valid as long as the buffer is an array.
cons:
- If the code gets into a function, and the buffer is then a pointer,
it will definitively produce a silent bug.
3.- Use sizeof() everywhere, and a magic number for the initializer.
The same as 2.
4.- Use ARRAY_BYTES() macro
pros:
- It is always safe and when code changes, it may break compilation, but
never a silent bug.
cons:
- Add a few lines of code. Maybe too much complexity for an example.
But I'd say that it is the only safe option, and in real code it
should probably be used more, so maybe it's good to show a good practice.
Here's my definition for ARRAY_BYTES(), which is makes use of
must_be_array() similar to the kernel ARRAY_SIZE():
4.1-
#define is_same_type(a, b) \
__builtin_types_compatible_p(__typeof__(a), __typeof__(b))
#define is_array(a) (!is_same_type((a), &(a)[0]))
#define must_be__(e, ...) ( \
0 * (int)sizeof( \
struct { \
_Static_assert((e) __VA_OPT__(,) __VA_ARGS__); \
char ISO_C_forbids_a_struct_with_no_members__; \
} \
) \
)
#define must_be_array__(a) must_be__(is_array(a), "Not an array!")
#define ARRAY_BYTES(arr) (sizeof(arr) + must_be_array__(arr))
The macro makes use of quite a few GNU extensions, though, which might
be too much to ask.
Actually, I was also going to propose this macro for the kernel itself,
to make it a bit safer.
There's a much simpler version of ARRAY_BYTES(), which requires the
macro to be defined in a header that is not a system header (to avoid
silencing warnings), and also requires a recent version of the compiler
to show a warning:
4.2-
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])
#define ARRAY_BYTES(arr) (sizeof((arr)[0]) * ARRAY_SIZE(arr))
What do you all think about the 5 different options? I don't know which
one is better.
sys/sysinfo.h:extern long int get_phys_pages (void)
for the real world i would say that long int == long but for the same reason
i would say what the include says and stay away from discussions.
jm2c,
wh
________________________________________
Von: [email protected] [[email protected]] im Auftrag von Alejandro Colomar [[email protected]]
Gesendet: Donnerstag, 10. September 2020 23:13
An: [email protected]
Cc: [email protected]; [email protected]; Alejandro Colomar
Betreff: [PATCH 17/24] get_phys_pages.3: Write 'long' instead of 'long int'
For consistency.
Most man pages use 'long' instead of 'long int'.
Signed-off-by: Alejandro Colomar <[email protected]>
---
man3/get_phys_pages.3 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man3/get_phys_pages.3 b/man3/get_phys_pages.3
index 4a9177dfd..97ba625b7 100644
--- a/man3/get_phys_pages.3
+++ b/man3/get_phys_pages.3
@@ -30,8 +30,8 @@ page counts
.nf
.B "#include <sys/sysinfo.h>"
.PP
-.B long int get_phys_pages(void);
-.B long int get_avphys_pages(void);
+.B long get_phys_pages(void);
+.B long get_avphys_pages(void);
.fi
.SH DESCRIPTION
The function
--
2.28.0
Hi,
On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
<[email protected]> wrote:
>
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
> man3/getgrent_r.3 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
> index 81d81a851..76deec370 100644
> --- a/man3/getgrent_r.3
> +++ b/man3/getgrent_r.3
> @@ -186,7 +186,7 @@ main(void)
>
> setgrent();
> while (1) {
> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
I'm worried that less attentive people might copy/paste parts of this
in their code, where maybe buf is just a pointer, and expect it to
work. Maybe leaving BUFLEN here is useful as a reminder that they need
to change something to adapt the code?
Just my 2 cents,
Stefan.
On 2020-09-11 17:28, Alejandro Colomar wrote:
> Hi Stefan,
>
> On 2020-09-11 16:35, Stefan Puiu wrote:
> > Hi,
> >
> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
> > <[email protected]> wrote:
> >>
> >> Signed-off-by: Alejandro Colomar <[email protected]>
> >> ---
> >> man3/getgrent_r.3 | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
> >> index 81d81a851..76deec370 100644
> >> --- a/man3/getgrent_r.3
> >> +++ b/man3/getgrent_r.3
> >> @@ -186,7 +186,7 @@ main(void)
> >>
> >> setgrent();
> >> while (1) {
> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
> >
> > I'm worried that less attentive people might copy/paste parts of this
> > in their code, where maybe buf is just a pointer, and expect it to
> > work. Maybe leaving BUFLEN here is useful as a reminder that they need
> > to change something to adapt the code?
> >
> > Just my 2 cents,
> > Stefan.
> >
> That's a very good point.
>
> So we have 3 options and I will propose now a 4th one. Let's see all
> of them and see which one is better for the man pages.
>
> 1.- Use the macro everywhere.
>
> pros:
> - It is still valid when the buffer is a pointer and not an array.
> cons:
> - Hardcodes the initializer. If the array is later initialized with a
> different value, it may produce a silent bug, or a compilation break.
>
> 2.- Use sizeof() everywhere, and the macro for the initializer.
>
> pros:
> - It is valid as long as the buffer is an array.
> cons:
> - If the code gets into a function, and the buffer is then a pointer,
> it will definitively produce a silent bug.
>
> 3.- Use sizeof() everywhere, and a magic number for the initializer.
>
> The same as 2.
>
> 4.- Use ARRAY_BYTES() macro
>
> pros:
> - It is always safe and when code changes, it may break compilation, but
> never a silent bug.
> cons:
> - Add a few lines of code. Maybe too much complexity for an example.
> But I'd say that it is the only safe option, and in real code it
> should probably be used more, so maybe it's good to show a good
> practice.
>
>
> Here's my definition for ARRAY_BYTES(), which is makes use of
> must_be_array() similar to the kernel ARRAY_SIZE():
>
> 4.1-
>
> #define is_same_type(a, b) \
> __builtin_types_compatible_p(__typeof__(a), __typeof__(b))
> #define is_array(a) (!is_same_type((a), &(a)[0]))
> #define must_be__(e, ...) ( \
> 0 * (int)sizeof( \
> struct { \
> _Static_assert((e) __VA_OPT__(,) __VA_ARGS__); \
> char ISO_C_forbids_a_struct_with_no_members__; \
> } \
> ) \
> )
> #define must_be_array__(a) must_be__(is_array(a), "Not an array!")
> #define ARRAY_BYTES(arr) (sizeof(arr) + must_be_array__(arr))
>
>
> The macro makes use of quite a few GNU extensions, though, which might
> be too much to ask.
>
> Actually, I was also going to propose this macro for the kernel itself,
> to make it a bit safer.
>
> There's a much simpler version of ARRAY_BYTES(), which requires the
> macro to be defined in a header that is not a system header (to avoid
> silencing warnings), and also requires a recent version of the compiler
> to show a warning:
>
> 4.2-
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])
> #define ARRAY_BYTES(arr) (sizeof((arr)[0]) * ARRAY_SIZE(arr))
>
>
> What do you all think about the 5 different options? I don't know which
> one is better.
I'd say 4.2 is the best one for the man pages. Just 2 one-line macro
definitions, very good safety, and pretty clear code.
Your thoughts?
the groff commands are ducument in man 7 groff
.nf No filling or adjusting of output-lines.
.fi Fill output lines
(for me) a typical use is like this:
.nf
struct timeval {
time_t tv_sec; /* seconds */
suseconds_t tv_usec; /* microseconds */
};
.fi
In the top section you prevent indenting (if any).
hth
wh
________________________________________
Von: [email protected] [[email protected]] im Auftrag von Alejandro Colomar [[email protected]]
Gesendet: Donnerstag, 10. September 2020 23:13
An: [email protected]
Cc: [email protected]; [email protected]; Alejandro Colomar
Betreff: [PATCH 22/24] membarrier.2: Note that glibc does not provide a wrapper
Notes: I copied .nf and .fi from futex.2, but they made no visual difference.
What do they actually do?
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/membarrier.2 | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/man2/membarrier.2 b/man2/membarrier.2
index 8825de71e..f65c6be5c 100644
--- a/man2/membarrier.2
+++ b/man2/membarrier.2
@@ -26,9 +26,15 @@
.SH NAME
membarrier \- issue memory barriers on a set of threads
.SH SYNOPSIS
+.nf
+.PP
.B #include <linux/membarrier.h>
.PP
.BI "int membarrier(int " cmd ", int " flags ");"
+.fi
+.PP
+.IR Note :
+There is no glibc wrapper for this system call; see NOTES.
.SH DESCRIPTION
The
.BR membarrier ()
@@ -270,6 +276,9 @@ Examples where
.BR membarrier ()
can be useful include implementations
of Read-Copy-Update libraries and garbage collectors.
+.PP
+Glibc does not provide a wrapper for this system call; call it using
+.BR syscall (2).
.SH EXAMPLES
Assuming a multithreaded application where "fast_path()" is executed
very frequently, and where "slow_path()" is executed infrequently, the
--
2.28.0
I do not think that this is a good idea.
An example should be clear and easy to understand.
(e.g. with reduced error handling)
These C99 macros are over the top for me.
jm2c
wh
________________________________________
Von: [email protected] [[email protected]] im Auftrag von Alejandro Colomar [[email protected]]
Gesendet: Donnerstag, 10. September 2020 23:13
An: [email protected]
Cc: [email protected]; [email protected]; Alejandro Colomar
Betreff: [PATCH 04/24] eventfd.2: Use 'PRIxN' macros when printing C99 fixed-width integer types
Signed-off-by: Alejandro Colomar <[email protected]>
---
man2/eventfd.2 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man2/eventfd.2 b/man2/eventfd.2
index 929234ab7..71e9d85b4 100644
--- a/man2/eventfd.2
+++ b/man2/eventfd.2
@@ -386,6 +386,7 @@ Parent read 28 (0x1c) from efd
.EX
#include <sys/eventfd.h>
#include <unistd.h>
+#include <inttypes.h> /* Definition of PRIu64 & PRIx64 */
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h> /* Definition of uint64_t */
@@ -430,8 +431,7 @@ main(int argc, char *argv[])
s = read(efd, &u, sizeof(uint64_t));
if (s != sizeof(uint64_t))
handle_error("read");
- printf("Parent read %llu (0x%llx) from efd\en",
- (unsigned long long) u, (unsigned long long) u);
+ printf("Parent read %"PRIu64" (0x%"PRIx64") from efd\en", u, u);
exit(EXIT_SUCCESS);
case \-1:
--
2.28.0
Hi Walter,
On 2020-09-11 14:50, Walter Harms wrote:
> BUFLEN should be remove completely or stay
>
> jm2c
> wh
Sorry that you weren't CC'd in the conversation we are having about it.
You can have a look at it here:
https://lore.kernel.org/linux-man/[email protected]/T/#m423de347de6a64d099887d4ce615660d16d5b0e6
I'll CC you in the next reply there.
Cheers,
Alex
On 2020-09-11 15:07, Walter Harms wrote:
>
>
> sys/sysinfo.h:extern long int get_phys_pages (void)
>
> for the real world i would say that long int == long but for the same reason
> i would say what the include says and stay away from discussions.
>
> jm2c,
> wh
I think that the man-pages don't need to follow other projects'
inconsistencies. 'long int' == 'long', so I think it's better to be
consistent here and hope that the other projects do that too.
A newbie may see 'long int' and 'long' used differently in the man-pages
and wonder that there might be a good reason for that to happen, and
won't understand why.
So I stand by this patch.
Cheers,
Alex
From: Michael Kerrisk
> Sent: 11 September 2020 10:31
>
> Hi Alex,
>
> On 9/10/20 11:13 PM, Alejandro Colomar wrote:
> > Signed-off-by: Alejandro Colomar <[email protected]>
> > ---
> > man3/inet_net_pton.3 | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/man3/inet_net_pton.3 b/man3/inet_net_pton.3
> > index 00f94b9d4..d74a33d74 100644
> > --- a/man3/inet_net_pton.3
> > +++ b/man3/inet_net_pton.3
> > @@ -332,6 +332,7 @@ Raw address: c1a80180
> > /* Link with "\-lresolv" */
> >
> > #include <arpa/inet.h>
> > +#include <inttypes.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> >
> > @@ -381,7 +382,7 @@ main(int argc, char *argv[])
> > may not have been touched by inet_net_ntop(), and so will still
> > have any initial value that was specified in argv[2]. */
> >
> > - printf("Raw address: %x\en", htonl(addr.s_addr));
> > + printf("Raw address: %"PRIx32"\en", htonl(addr.s_addr));
> >
> > exit(EXIT_SUCCESS);
> > }
>
> So, I'm in a little bit of doubt about patches 01 and 02. Does
> this really win us anything? On the one hand, %"PRIx32" is more
> difficult to read than %x. On the other, does it win us anything
> in terms of portability? At first glance, the answers seems to me
> to be "no". Your thoughts?
On 32bit systems uint32_t might be either 'int' or 'long'.
So the format has to match - even though the ABI is probably
the same in both cases.
Mind you, htonl() itself could be problematic.
On BE systems it is likely to be #define htonl(x) (x)
so the type is whatever was passed.
On LE systems it might even be long htonl(long)
- which is its historic prototype.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
missunderstanding,
i do not want to discuss sizeof() vs define
in this example you do:
#define BUFLEN 4096
char buf[BUFLEN];
getgrent_r(&grp, buf, sizeof(buf), &grpp);
so there is no real need for BUFLEN anymore
so donig
char buf[BUFLEN]; -> char buf[4096];
would remove BUFLEN
________________________________________
Von: Alejandro Colomar [[email protected]]
Gesendet: Freitag, 11. September 2020 21:17
An: Walter Harms; [email protected]
Cc: [email protected]; [email protected]
Betreff: Re: AW: [PATCH 12/24] getgrent_r.3: Use sizeof() to get buffer size (instead of hardcoding macro name)
Hi Walter,
On 2020-09-11 14:50, Walter Harms wrote:
> BUFLEN should be remove completely or stay
>
> jm2c
> wh
Sorry that you weren't CC'd in the conversation we are having about it.
You can have a look at it here:
https://lore.kernel.org/linux-man/[email protected]/T/#m423de347de6a64d099887d4ce615660d16d5b0e6
I'll CC you in the next reply there.
Cheers,
Alex
Hi Walter,
On 9/14/20 11:24 AM, Walter Harms wrote:
> missunderstanding,
> i do not want to discuss sizeof() vs define
I did understand, that was point (3).
>
> in this example you do:
> #define BUFLEN 4096
> char buf[BUFLEN];
> getgrent_r(&grp, buf, sizeof(buf), &grpp);
>
> so there is no real need for BUFLEN anymore
> so donig
> char buf[BUFLEN]; -> char buf[4096];
> would remove BUFLEN
>
However, I think that might be better as a separate patch.
I really don't have many arguments for nor against it.
Thanks,
Alex
Hi,
On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar
<[email protected]> wrote:
>
> Hi Stefan,
>
> On 2020-09-11 16:35, Stefan Puiu wrote:
> > Hi,
> >
> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
> > <[email protected]> wrote:
> >>
> >> Signed-off-by: Alejandro Colomar <[email protected]>
> >> ---
> >> man3/getgrent_r.3 | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
> >> index 81d81a851..76deec370 100644
> >> --- a/man3/getgrent_r.3
> >> +++ b/man3/getgrent_r.3
> >> @@ -186,7 +186,7 @@ main(void)
> >>
> >> setgrent();
> >> while (1) {
> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
> >
> > I'm worried that less attentive people might copy/paste parts of this
> > in their code, where maybe buf is just a pointer, and expect it to
> > work. Maybe leaving BUFLEN here is useful as a reminder that they need
> > to change something to adapt the code?
> >
> > Just my 2 cents,
> > Stefan.
> >
> That's a very good point.
>
> So we have 3 options and I will propose now a 4th one. Let's see all
> of them and see which one is better for the man pages.
>
> 1.- Use the macro everywhere.
>
> pros:
> - It is still valid when the buffer is a pointer and not an array.
> cons:
> - Hardcodes the initializer. If the array is later initialized with a
> different value, it may produce a silent bug, or a compilation break.
>
> 2.- Use sizeof() everywhere, and the macro for the initializer.
>
> pros:
> - It is valid as long as the buffer is an array.
> cons:
> - If the code gets into a function, and the buffer is then a pointer,
> it will definitively produce a silent bug.
>
> 3.- Use sizeof() everywhere, and a magic number for the initializer.
>
> The same as 2.
>
> 4.- Use ARRAY_BYTES() macro
>
> pros:
> - It is always safe and when code changes, it may break compilation, but
> never a silent bug.
> cons:
> - Add a few lines of code. Maybe too much complexity for an example.
> But I'd say that it is the only safe option, and in real code it
> should probably be used more, so maybe it's good to show a good practice.
If you ask me, I think examples should be simple and easy to
understand, and easy to copy/paste in your code. I'd settle for easy
enough, not perfect or completely foolproof. If you need to look up
obscure gcc features to understand an example, that's not very
helpful. So I'd be more inclined to prefer version 1 above. But let's
see Michael's opinion on this.
Just my 2c,
Stefan.
At 2020-09-11T12:58:08+0000, Walter Harms wrote:
> the groff commands are ducument in man 7 groff
> .nf No filling or adjusting of output-lines.
> .fi Fill output lines
>
> (for me) a typical use is like this:
> .nf
>
> struct timeval {
> time_t tv_sec; /* seconds */
> suseconds_t tv_usec; /* microseconds */
> };
> .fi
>
> In the top section you prevent indenting (if any).
The above will not work as desired for typesetter output, a.k.a., "troff
devices", such as PostScript or PDF. The initial code indent might work
okay but the alignment of the field names will become
ragged/mis-registered and the comments even more so.
This is because a proportional font is used by default for troff
devices. The classical man macros, going back to Version 7 Unix (1979)
had no good solution for this problem and Unix room tradition at Murray
Hill going all the way back to (what we now call) the First Edition
manual in 1971 was to read the man pages on a typewriter--a Teletype
Model 33 or Model 37. Typewriters, of course, always[1] used monospaced
fonts.
Version 9 Unix (1986) introduced .EX and .EE for setting material in a
monospaced font even if the device used proportional type by default.
(Plan 9 troff inherited them.) GNU roff has supporteds .EX and .EE as
well, for over 13 years, and its implementations are ultra-permissively
licensed so other *roffs like Heirloom Doctools have picked them up.
Therefore I recommend .EX and .EE for all code examples.
They are very simple to use. In the above, simply replace ".nf" with
".EX" and ".fi" with ".EE".
Regards,
Branden
[1] Not completely true; variable-pitch typewriters (such as 10/12 point
selectable) were fairly common and some expensive models like the IBM
Executive even featured true proportional type.
On 9/15/20 12:03 PM, Stefan Puiu wrote:
> Hi,
>
> On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar
> <[email protected]> wrote:
>>
>> Hi Stefan,
>>
>> On 2020-09-11 16:35, Stefan Puiu wrote:
>> > Hi,
>> >
>> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
>> > <[email protected]> wrote:
>> >>
>> >> Signed-off-by: Alejandro Colomar <[email protected]>
>> >> ---
>> >> man3/getgrent_r.3 | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
>> >> index 81d81a851..76deec370 100644
>> >> --- a/man3/getgrent_r.3
>> >> +++ b/man3/getgrent_r.3
>> >> @@ -186,7 +186,7 @@ main(void)
>> >>
>> >> setgrent();
>> >> while (1) {
>> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
>> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
>> >
>> > I'm worried that less attentive people might copy/paste parts of this
>> > in their code, where maybe buf is just a pointer, and expect it to
>> > work. Maybe leaving BUFLEN here is useful as a reminder that they need
>> > to change something to adapt the code?
>> >
>> > Just my 2 cents,
>> > Stefan.
>> >
>> That's a very good point.
>>
>> So we have 3 options and I will propose now a 4th one. Let's see all
>> of them and see which one is better for the man pages.
>>
>> 1.- Use the macro everywhere.
>>
>> pros:
>> - It is still valid when the buffer is a pointer and not an array.
>> cons:
>> - Hardcodes the initializer. If the array is later initialized with a
>> different value, it may produce a silent bug, or a compilation break.
>>
>> 2.- Use sizeof() everywhere, and the macro for the initializer.
>>
>> pros:
>> - It is valid as long as the buffer is an array.
>> cons:
>> - If the code gets into a function, and the buffer is then a pointer,
>> it will definitively produce a silent bug.
>>
>> 3.- Use sizeof() everywhere, and a magic number for the initializer.
>>
>> The same as 2.
>>
>> 4.- Use ARRAY_BYTES() macro
>>
>> pros:
>> - It is always safe and when code changes, it may break compilation, but
>> never a silent bug.
>> cons:
>> - Add a few lines of code. Maybe too much complexity for an example.
>> But I'd say that it is the only safe option, and in real code it
>> should probably be used more, so maybe it's good to show a good practice.
>
> If you ask me, I think examples should be simple and easy to
> understand, and easy to copy/paste in your code. I'd settle for easy
> enough, not perfect or completely foolproof. If you need to look up
> obscure gcc features to understand an example, that's not very
> helpful. So I'd be more inclined to prefer version 1 above. But let's
> see Michael's opinion on this.
>
> Just my 2c,
So, the fundamental problem is that C is nearly 50 years old.
It's a great high-level assembly language, but when it comes
to nuances like this it gets pretty painful. One can do macro
magic of the kind you suggest, but I agree with Stefan that it
gets confusing and distracting for the reader. I think I also
lean to solution 1. Yes, it's not perfect, but it's easy to
understand, and I don't think we can or should try and solve
the broken-ness of C in the manual pages.
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Branden,
On 9/21/20 4:36 PM, G. Branden Robinson wrote:
> At 2020-09-11T12:58:08+0000, Walter Harms wrote:
>> the groff commands are ducument in man 7 groff
>> .nf No filling or adjusting of output-lines.
>> .fi Fill output lines
>>
>> (for me) a typical use is like this:
>> .nf
>>
>> struct timeval {
>> time_t tv_sec; /* seconds */
>> suseconds_t tv_usec; /* microseconds */
>> };
>> .fi
>>
>> In the top section you prevent indenting (if any).
>
> The above will not work as desired for typesetter output, a.k.a., "troff
> devices", such as PostScript or PDF. The initial code indent might work
> okay but the alignment of the field names will become
> ragged/mis-registered and the comments even more so.
Yes.
> This is because a proportional font is used by default for troff
> devices. The classical man macros, going back to Version 7 Unix (1979)
> had no good solution for this problem and Unix room tradition at Murray
> Hill going all the way back to (what we now call) the First Edition
> manual in 1971 was to read the man pages on a typewriter--a Teletype
> Model 33 or Model 37. Typewriters, of course, always[1] used monospaced
> fonts.
>
> Version 9 Unix (1986) introduced .EX and .EE for setting material in a
> monospaced font even if the device used proportional type by default.
> (Plan 9 troff inherited them.) GNU roff has supporteds .EX and .EE as
> well, for over 13 years, and its implementations are ultra-permissively
> licensed so other *roffs like Heirloom Doctools have picked them up.
> Therefore I recommend .EX and .EE for all code examples.
>
> They are very simple to use. In the above, simply replace ".nf" with
> ".EX" and ".fi" with ".EE".
>
> Regards,
> Branden
>
> [1] Not completely true; variable-pitch typewriters (such as 10/12 point
> selectable) were fairly common and some expensive models like the IBM
> Executive even featured true proportional type.
Thanks for the interesting history, Branden!
From time toi time I wonder if the function prototypes in
the SYNOPSIS should also be inside .EX/.EE. Your thoughts?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi,
On 2020-09-23 22:35, Michael Kerrisk (man-pages) wrote:
> On 9/15/20 12:03 PM, Stefan Puiu wrote:
>> Hi,
>>
>> On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar
>> <[email protected]> wrote:
>>>
>>> Hi Stefan,
>>>
>>> On 2020-09-11 16:35, Stefan Puiu wrote:
>>> > Hi,
>>> >
>>> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
>>> > <[email protected]> wrote:
>>> >>
>>> >> Signed-off-by: Alejandro Colomar <[email protected]>
>>> >> ---
>>> >> man3/getgrent_r.3 | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
>>> >> index 81d81a851..76deec370 100644
>>> >> --- a/man3/getgrent_r.3
>>> >> +++ b/man3/getgrent_r.3
>>> >> @@ -186,7 +186,7 @@ main(void)
>>> >>
>>> >> setgrent();
>>> >> while (1) {
>>> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
>>> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
>>> >
>>> > I'm worried that less attentive people might copy/paste parts of this
>>> > in their code, where maybe buf is just a pointer, and expect it to
>>> > work. Maybe leaving BUFLEN here is useful as a reminder that they need
>>> > to change something to adapt the code?
>>> >
>>> > Just my 2 cents,
>>> > Stefan.
>>> >
>>> That's a very good point.
>>>
>>> So we have 3 options and I will propose now a 4th one. Let's see all
>>> of them and see which one is better for the man pages.
>>>
>>> 1.- Use the macro everywhere.
>>>
>>> pros:
>>> - It is still valid when the buffer is a pointer and not an array.
>>> cons:
>>> - Hardcodes the initializer. If the array is later initialized with a
>>> different value, it may produce a silent bug, or a compilation break.
>>>
>>> 2.- Use sizeof() everywhere, and the macro for the initializer.
>>>
>>> pros:
>>> - It is valid as long as the buffer is an array.
>>> cons:
>>> - If the code gets into a function, and the buffer is then a pointer,
>>> it will definitively produce a silent bug.
>>>
>>> 3.- Use sizeof() everywhere, and a magic number for the initializer.
>>>
>>> The same as 2.
>>>
>>> 4.- Use ARRAY_BYTES() macro
>>>
>>> pros:
>>> - It is always safe and when code changes, it may break compilation, but
>>> never a silent bug.
>>> cons:
>>> - Add a few lines of code. Maybe too much complexity for an example.
>>> But I'd say that it is the only safe option, and in real code it
>>> should probably be used more, so maybe it's good to show a good practice.
>>
>> If you ask me, I think examples should be simple and easy to
>> understand, and easy to copy/paste in your code. I'd settle for easy
>> enough, not perfect or completely foolproof. If you need to look up
>> obscure gcc features to understand an example, that's not very
>> helpful. So I'd be more inclined to prefer version 1 above. But let's
>> see Michael's opinion on this.
>>
>> Just my 2c,
>
> So, the fundamental problem is that C is nearly 50 years old.
> It's a great high-level assembly language, but when it comes
> to nuances like this it gets pretty painful. One can do macro
> magic of the kind you suggest, but I agree with Stefan that it
> gets confusing and distracting for the reader. I think I also
> lean to solution 1. Yes, it's not perfect, but it's easy to
> understand, and I don't think we can or should try and solve
> the broken-ness of C in the manual pages.
>
> Thanks,
>
> Michael
>
>
I was reverting the 3 patches I introduced (they changed from solution 1
to solution 2), and also was grepping for already existing solution 2 in
the pages (it seems that solution 2 was a bit more extended than
solution 1).
While doing that, I've been thinking about it again...
There's a good thing about sizeof (even though I admit it's very
insecure; and I never use it for myself), especially for the man pages:
I'll copy here a sample from getnameinfo.3 to ilustrate it:
[[
.EX
struct sockaddr *addr; /* input */
socklen_t addrlen; /* input */
char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
if (getnameinfo(addr, addrlen, hbuf, sizeof(hbuf), sbuf,
sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV) == 0)
printf("host=%s, serv=%s\en", hbuf, sbuf);
.EE
]]
Here, it's clear to the reader that the 4th argument to 'getnameinfo()'
is the size of the buffer passed as the 3rd argument.
If the function call was changed to
[[
getnameinfo(addr, addrlen, hbuf, NI_MAXHOST, sbuf,
sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV)
]]
then it would be less clear, and the reader should go back and forth to
see where that comes from. In this short example it is relatively very
clear, but in some examples it might be less clear.
Would you maintain your preference for solution 1?
Also... I am trying to patch glibc to provide a safe version of
'nitems()', and shortly after they accept that patch (if they do), I'll
send another one to add a safe 'array_bytes()' based on 'nitems()'.
Maybe the examples could use 'array_bytes()'; although is will be a
glibc extension, and non-existent in any other POSIX systems, of course,
which would make the examples non-portable, but still can be solved with
a simple
[[
#if !defined(array_bytes)
#define array_bytes() sizeof()
#endif
]]
But again it complicates the examples...
I'm not sure at all about what should be done. Please comment. If you
still prefer solution 1, I'll send you a patch with the revert + fixes,
but I think it's very delicate.
Thanks,
Alex
Hi Alex,
[..]
> I was reverting the 3 patches I introduced (they changed from solution 1
> to solution 2), and also was grepping for already existing solution 2 in
> the pages (it seems that solution 2 was a bit more extended than
> solution 1).
Just so I can refresh my cache, which commits were those?
Thanks,
Michael
Hi Michael
On 2020-09-24 12:04, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
>
> [..]
>
>> I was reverting the 3 patches I introduced (they changed from solution 1
>> to solution 2), and also was grepping for already existing solution 2 in
>> the pages (it seems that solution 2 was a bit more extended than
>> solution 1).
>
> Just so I can refresh my cache, which commits were those?
* b9bf90297 - Thu, 10 Sep 2020 23:13:36 +0200 (2 weeks ago)
| unix.7: Use sizeof() to get buffer size (instead of
hardcoding macro name) - Alejandro Colomar
[...]
* 1656c1702 - Thu, 10 Sep 2020 23:13:34 +0200 (2 weeks ago)
| getpwent_r.3: Use sizeof() to get buffer size (instead of
hardcoding macro name) - Alejandro Colomar
* cf254328f - Thu, 10 Sep 2020 23:13:33 +0200 (2 weeks ago)
| getgrent_r.3: Use sizeof() to get buffer size (instead of
hardcoding macro name) - Alejandro Colomar
>
> Thanks,
>
> Michael
>
Cheers,
Alex
On 9/24/20 11:35 AM, Alejandro Colomar wrote:
> Hi,
>
> On 2020-09-23 22:35, Michael Kerrisk (man-pages) wrote:
>> On 9/15/20 12:03 PM, Stefan Puiu wrote:
>>> Hi,
>>>
>>> On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar
>>> <[email protected]> wrote:
>>>>
>>>> Hi Stefan,
>>>>
>>>> On 2020-09-11 16:35, Stefan Puiu wrote:
>>>> > Hi,
>>>> >
>>>> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
>>>> > <[email protected]> wrote:
>>>> >>
>>>> >> Signed-off-by: Alejandro Colomar <[email protected]>
>>>> >> ---
>>>> >> man3/getgrent_r.3 | 2 +-
>>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> >>
>>>> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
>>>> >> index 81d81a851..76deec370 100644
>>>> >> --- a/man3/getgrent_r.3
>>>> >> +++ b/man3/getgrent_r.3
>>>> >> @@ -186,7 +186,7 @@ main(void)
>>>> >>
>>>> >> setgrent();
>>>> >> while (1) {
>>>> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
>>>> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
>>>> >
>>>> > I'm worried that less attentive people might copy/paste parts of this
>>>> > in their code, where maybe buf is just a pointer, and expect it to
>>>> > work. Maybe leaving BUFLEN here is useful as a reminder that they need
>>>> > to change something to adapt the code?
>>>> >
>>>> > Just my 2 cents,
>>>> > Stefan.
>>>> >
>>>> That's a very good point.
>>>>
>>>> So we have 3 options and I will propose now a 4th one. Let's see all
>>>> of them and see which one is better for the man pages.
>>>>
>>>> 1.- Use the macro everywhere.
>>>>
>>>> pros:
>>>> - It is still valid when the buffer is a pointer and not an array.
>>>> cons:
>>>> - Hardcodes the initializer. If the array is later initialized with a
>>>> different value, it may produce a silent bug, or a compilation break.
>>>>
>>>> 2.- Use sizeof() everywhere, and the macro for the initializer.
>>>>
>>>> pros:
>>>> - It is valid as long as the buffer is an array.
>>>> cons:
>>>> - If the code gets into a function, and the buffer is then a pointer,
>>>> it will definitively produce a silent bug.
>>>>
>>>> 3.- Use sizeof() everywhere, and a magic number for the initializer.
>>>>
>>>> The same as 2.
>>>>
>>>> 4.- Use ARRAY_BYTES() macro
>>>>
>>>> pros:
>>>> - It is always safe and when code changes, it may break compilation, but
>>>> never a silent bug.
>>>> cons:
>>>> - Add a few lines of code. Maybe too much complexity for an example.
>>>> But I'd say that it is the only safe option, and in real code it
>>>> should probably be used more, so maybe it's good to show a good practice.
>>>
>>> If you ask me, I think examples should be simple and easy to
>>> understand, and easy to copy/paste in your code. I'd settle for easy
>>> enough, not perfect or completely foolproof. If you need to look up
>>> obscure gcc features to understand an example, that's not very
>>> helpful. So I'd be more inclined to prefer version 1 above. But let's
>>> see Michael's opinion on this.
>>>
>>> Just my 2c,
>>
>> So, the fundamental problem is that C is nearly 50 years old.
>> It's a great high-level assembly language, but when it comes
>> to nuances like this it gets pretty painful. One can do macro
>> magic of the kind you suggest, but I agree with Stefan that it
>> gets confusing and distracting for the reader. I think I also
>> lean to solution 1. Yes, it's not perfect, but it's easy to
>> understand, and I don't think we can or should try and solve
>> the broken-ness of C in the manual pages.
>>
>> Thanks,
>>
>> Michael
>>
>>
>
> I was reverting the 3 patches I introduced (they changed from solution 1
> to solution 2), and also was grepping for already existing solution 2 in
> the pages (it seems that solution 2 was a bit more extended than
> solution 1).
>
> While doing that, I've been thinking about it again...
>
> There's a good thing about sizeof (even though I admit it's very
> insecure; and I never use it for myself), especially for the man pages:
>
> I'll copy here a sample from getnameinfo.3 to ilustrate it:
>
> [[
> .EX
> struct sockaddr *addr; /* input */
> socklen_t addrlen; /* input */
> char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
>
> if (getnameinfo(addr, addrlen, hbuf, sizeof(hbuf), sbuf,
> sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV) == 0)
> printf("host=%s, serv=%s\en", hbuf, sbuf);
> .EE
> ]]
>
> Here, it's clear to the reader that the 4th argument to 'getnameinfo()'
> is the size of the buffer passed as the 3rd argument.
>
> If the function call was changed to
>
> [[
> getnameinfo(addr, addrlen, hbuf, NI_MAXHOST, sbuf,
> sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV)
> ]]
>
> then it would be less clear, and the reader should go back and forth to
> see where that comes from. In this short example it is relatively very
> clear, but in some examples it might be less clear.
>
> Would you maintain your preference for solution 1?
>
>
> Also... I am trying to patch glibc to provide a safe version of
> 'nitems()', and shortly after they accept that patch (if they do), I'll
> send another one to add a safe 'array_bytes()' based on 'nitems()'.
>
> Maybe the examples could use 'array_bytes()'; although is will be a
> glibc extension, and non-existent in any other POSIX systems, of course,
> which would make the examples non-portable, but still can be solved with
> a simple
>
> [[
> #if !defined(array_bytes)
> #define array_bytes() sizeof()
> #endif
> ]]
>
> But again it complicates the examples...
(And I'd rather not...)
> I'm not sure at all about what should be done. Please comment. If you
> still prefer solution 1, I'll send you a patch with the revert + fixes,
> but I think it's very delicate.
Okay -- I agree with your perspective of the getnameinfo example.
So, I think maybe solution 1 is clearer sometimes, and other times
solution 2 is clearer. I don't feel too strongly about this
(because we can't solve the bugger problem, which is C).
I'm fine with not reverting the three patches you
refer to. I'm going to leave this decision to you :-)!
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
Hi Michael,
On 2020-09-24 13:38, Michael Kerrisk (man-pages) wrote:
> On 9/24/20 11:35 AM, Alejandro Colomar wrote:
>> Hi,
>>
>> On 2020-09-23 22:35, Michael Kerrisk (man-pages) wrote:
>>> On 9/15/20 12:03 PM, Stefan Puiu wrote:
>>>> Hi,
>>>>
>>>> On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar
>>>> <[email protected]> wrote:
>>>>>
>>>>> Hi Stefan,
>>>>>
>>>>> On 2020-09-11 16:35, Stefan Puiu wrote:
>>>>> > Hi,
>>>>> >
>>>>> > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar
>>>>> > <[email protected]> wrote:
>>>>> >>
>>>>> >> Signed-off-by: Alejandro Colomar <[email protected]>
>>>>> >> ---
>>>>> >> man3/getgrent_r.3 | 2 +-
>>>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> >>
>>>>> >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
>>>>> >> index 81d81a851..76deec370 100644
>>>>> >> --- a/man3/getgrent_r.3
>>>>> >> +++ b/man3/getgrent_r.3
>>>>> >> @@ -186,7 +186,7 @@ main(void)
>>>>> >>
>>>>> >> setgrent();
>>>>> >> while (1) {
>>>>> >> - i = getgrent_r(&grp, buf, BUFLEN, &grpp);
>>>>> >> + i = getgrent_r(&grp, buf, sizeof(buf), &grpp);
>>>>> >
>>>>> > I'm worried that less attentive people might copy/paste
parts of this
>>>>> > in their code, where maybe buf is just a pointer, and expect
it to
>>>>> > work. Maybe leaving BUFLEN here is useful as a reminder that
they need
>>>>> > to change something to adapt the code?
>>>>> >
>>>>> > Just my 2 cents,
>>>>> > Stefan.
>>>>> >
>>>>> That's a very good point.
>>>>>
>>>>> So we have 3 options and I will propose now a 4th one. Let's see all
>>>>> of them and see which one is better for the man pages.
>>>>>
>>>>> 1.- Use the macro everywhere.
>>>>>
>>>>> pros:
>>>>> - It is still valid when the buffer is a pointer and not an array.
>>>>> cons:
>>>>> - Hardcodes the initializer. If the array is later initialized
with a
>>>>> different value, it may produce a silent bug, or a
compilation break.
>>>>>
>>>>> 2.- Use sizeof() everywhere, and the macro for the initializer.
>>>>>
>>>>> pros:
>>>>> - It is valid as long as the buffer is an array.
>>>>> cons:
>>>>> - If the code gets into a function, and the buffer is then a pointer,
>>>>> it will definitively produce a silent bug.
>>>>>
>>>>> 3.- Use sizeof() everywhere, and a magic number for the
initializer.
>>>>>
>>>>> The same as 2.
>>>>>
>>>>> 4.- Use ARRAY_BYTES() macro
>>>>>
>>>>> pros:
>>>>> - It is always safe and when code changes, it may break
compilation, but
>>>>> never a silent bug.
>>>>> cons:
>>>>> - Add a few lines of code. Maybe too much complexity for an example.
>>>>> But I'd say that it is the only safe option, and in real code it
>>>>> should probably be used more, so maybe it's good to show a
good practice.
>>>>
>>>> If you ask me, I think examples should be simple and easy to
>>>> understand, and easy to copy/paste in your code. I'd settle for easy
>>>> enough, not perfect or completely foolproof. If you need to look up
>>>> obscure gcc features to understand an example, that's not very
>>>> helpful. So I'd be more inclined to prefer version 1 above. But let's
>>>> see Michael's opinion on this.
>>>>
>>>> Just my 2c,
>>>
>>> So, the fundamental problem is that C is nearly 50 years old.
>>> It's a great high-level assembly language, but when it comes
>>> to nuances like this it gets pretty painful. One can do macro
>>> magic of the kind you suggest, but I agree with Stefan that it
>>> gets confusing and distracting for the reader. I think I also
>>> lean to solution 1. Yes, it's not perfect, but it's easy to
>>> understand, and I don't think we can or should try and solve
>>> the broken-ness of C in the manual pages.
>>>
>>> Thanks,
>>>
>>> Michael
>>>
>>>
>>
>> I was reverting the 3 patches I introduced (they changed from solution 1
>> to solution 2), and also was grepping for already existing solution 2 in
>> the pages (it seems that solution 2 was a bit more extended than
>> solution 1).
>>
>> While doing that, I've been thinking about it again...
>>
>> There's a good thing about sizeof (even though I admit it's very
>> insecure; and I never use it for myself), especially for the man pages:
>>
>> I'll copy here a sample from getnameinfo.3 to ilustrate it:
>>
>> [[
>> .EX
>> struct sockaddr *addr; /* input */
>> socklen_t addrlen; /* input */
>> char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
>>
>> if (getnameinfo(addr, addrlen, hbuf, sizeof(hbuf), sbuf,
>> sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV) == 0)
>> printf("host=%s, serv=%s\en", hbuf, sbuf);
>> .EE
>> ]]
>>
>> Here, it's clear to the reader that the 4th argument to 'getnameinfo()'
>> is the size of the buffer passed as the 3rd argument.
>>
>> If the function call was changed to
>>
>> [[
>> getnameinfo(addr, addrlen, hbuf, NI_MAXHOST, sbuf,
>> sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV)
>> ]]
>>
>> then it would be less clear, and the reader should go back and forth to
>> see where that comes from. In this short example it is relatively very
>> clear, but in some examples it might be less clear.
>>
>> Would you maintain your preference for solution 1?
>>
>>
>> Also... I am trying to patch glibc to provide a safe version of
>> 'nitems()', and shortly after they accept that patch (if they do), I'll
>> send another one to add a safe 'array_bytes()' based on 'nitems()'.
>>
>> Maybe the examples could use 'array_bytes()'; although is will be a
>> glibc extension, and non-existent in any other POSIX systems, of course,
>> which would make the examples non-portable, but still can be solved with
>> a simple
>>
>> [[
>> #if !defined(array_bytes)
>> #define array_bytes() sizeof()
>> #endif
>> ]]
>>
>> But again it complicates the examples...
>
> (And I'd rather not...)
>
>> I'm not sure at all about what should be done. Please comment. If you
>> still prefer solution 1, I'll send you a patch with the revert + fixes,
>> but I think it's very delicate.
>
> Okay -- I agree with your perspective of the getnameinfo example.
>
> So, I think maybe solution 1 is clearer sometimes, and other times
> solution 2 is clearer. I don't feel too strongly about this
Me neither.
> (because we can't solve the bugger problem, which is C).
> I'm fine with not reverting the three patches you
> refer to. I'm going to leave this decision to you :-)!
>
> Thanks,
>
> Michael
>
>
I'll keep it as is now. :)
Thanks,
Alex
At 2020-09-24T10:06:23+0200, Michael Kerrisk (man-pages) wrote:
> Thanks for the interesting history, Branden!
Hi, Michael. And you're welcome! I often wonder if I test people's
patience with my info dumps but I try to show my work when making
claims.
> From time toi time I wonder if the function prototypes in
> the SYNOPSIS should also be inside .EX/.EE. Your thoughts?
I think there are trade-offs.
1. If you want alignment, the monospaced font that .EX/.EE uses is the
most portable way to get it.
2. For typeset output, you'll generally run out of line more quickly
with a monospaced font than with the troff/man default (Times).
_Any_ time filling is off, output should be checked to see if it
overruns the right margin, but this point strengthens in monospace.
Here's something that isn't a trade-off that might come as a surprise to
some readers.
* You can still get bold and italics inside an .EX/.EE region, so you
can still use these distinguish data types, variable names, and
what-have-you.
The idiom for achieving this is apparently not well-known among those
who write man pages by hand, and tools that generate man(7) language
from some other source often produce output that is so ugly as to be
unintelligible to non-experts in *roff.
The key insights are that (A) you can still make macro calls inside an
.EX/.EE region, and (B) you can use the \c escape to "interrupt" an
input line and continue it on the next without introducing any
whitespace. For instance, looking at fstat() from your stat(2) page, I
might write it using .EX and .EE as follows:
.EX
.B int fstat(int \c
.IB fd , \~\c
.B struct stat *\c
.IB statbuf );
.EE
Normally in man pages, it is senseless to have any spaces before the \c
escape, and \c is best omitted in that event. However, when filling is
disabled (as is the case in .EX/.EE regions), output lines break where
the input lines do by default--\c overrides this, causing the lines to
be joined. (Regarding the \~, see below.)
If there is no use for roman in the line, then you could do the whole
function signature with the .BI macro by quoting macro arguments that
contain whitespace.
.EX
.BI "int fstat(int " fd ", struct stat *" statbuf );
.EE
As a matter of personal style, I find quoted space characters interior
but adjacent to quotation marks visually confusing--it's slower for me
to tell which parts of the line are "inside" the quotes and which
outside--so I turn to groff's \~ non-breaking space escape (widely
supported elsewhere) for these boundary spaces.
.EX
.BI "int fstat(int\~" fd ", struct stat *" statbuf );
.EE
Which of the above three models do you think would work best for the
man-pages project?
Also, do you have any use for roman in function signatures? I see it
used for comments and feature test macro material, but not within
function signatures proper.
As an aside, I will admit to some unease with the heavy use of bold in
synopses in section 2 and 3 man pages, but I can marshal no historical
argument against it. In fact, a quick check of some Unix v7 section 2
pages from 1979 that I have lying around (thanks to TUHS) reveals that
Bell Labs used undifferentiated bold for the whole synopsis!
$ head -n 13 usr/man/man2/stat.2
.TH STAT 2
.SH NAME
stat, fstat \- get file status
.SH SYNOPSIS
.B #include <sys/types.h>
.br
.B #include <sys/stat.h>
.PP
.B stat(name, buf)
.br
.B char *name;
.br
.B struct stat *buf;
Regards,
Branden
Hi Branden,
* G. Branden Robinson via linux-man:
1)
> .EX
> .B int fstat(int \c
> .IB fd , \~\c
> .B struct stat *\c
> .IB statbuf );
> .EE
2)
> .EX
> .BI "int fstat(int " fd ", struct stat *" statbuf );
> .EE
3)
> .EX
> .BI "int fstat(int\~" fd ", struct stat *" statbuf );
> .EE
I'd say number 2 is best. Rationale: grep :)
I agree it's visually somewhat harder, but grepping is way easier.
Regards,
Alex
At 2020-09-27T22:05:14+0200, Alejandro Colomar wrote:
> Hi Branden,
>
> * G. Branden Robinson via linux-man:
>
> 1)
>
> > .EX
> > .B int fstat(int \c
> > .IB fd , \~\c
> > .B struct stat *\c
> > .IB statbuf );
> > .EE
>
> 2)
>
> > .EX
> > .BI "int fstat(int " fd ", struct stat *" statbuf );
> > .EE
>
> 3)
>
> > .EX
> > .BI "int fstat(int\~" fd ", struct stat *" statbuf );
> > .EE
>
> I'd say number 2 is best. Rationale: grep :)
> I agree it's visually somewhat harder, but grepping is way easier.
I don't see how (2) is any tougher to grep than (3)...?
If I'm grepping, I'm usually concerned with things like
variable/function names and not with punctuation, so if I were grepping
for the above function signature I'd probably write:
$ grep 'fstat.*fd.*statbuf' man2/*
...which would catch either of the above just fine.
Am I missing something?
Regards,
Branden
On 2020-09-28 14:52, G. Branden Robinson wrote:
> At 2020-09-27T22:05:14+0200, Alejandro Colomar wrote:
>> Hi Branden,
>>
>> * G. Branden Robinson via linux-man:
>>
>> 1)
>>
>>> .EX
>>> .B int fstat(int \c
>>> .IB fd , \~\c
>>> .B struct stat *\c
>>> .IB statbuf );
>>> .EE
>>
>> 2)
>>
>>> .EX
>>> .BI "int fstat(int " fd ", struct stat *" statbuf );
>>> .EE
>>
>> 3)
>>
>>> .EX
>>> .BI "int fstat(int\~" fd ", struct stat *" statbuf );
>>> .EE
>>
>> I'd say number 2 is best. Rationale: grep :)
>> I agree it's visually somewhat harder, but grepping is way easier.
>
> I don't see how (2) is any tougher to grep than (3)...?
>
> If I'm grepping, I'm usually concerned with things like
> variable/function names and not with punctuation, so if I were grepping
> for the above function signature I'd probably write:
>
> $ grep 'fstat.*fd.*statbuf' man2/*
>
> ...which would catch either of the above just fine.
>
> Am I missing something?
>
> Regards,
> Branden
>
There are a few cases: if I want to find declarations of type int,
I'd start with:
$ grep -rn "int\s"
or something like that. "int\~" would break the ability to do that.
Regards,
Alex
At 2020-09-28T15:33:21+0200, Alejandro Colomar wrote:
> On 2020-09-28 14:52, G. Branden Robinson wrote:
> > At 2020-09-27T22:05:14+0200, Alejandro Colomar wrote:
> >> 2)
> >>
> >>> .EX
> >>> .BI "int fstat(int " fd ", struct stat *" statbuf );
> >>> .EE
> >>
> >> 3)
> >>
> >>> .EX
> >>> .BI "int fstat(int\~" fd ", struct stat *" statbuf );
> >>> .EE
> >>
> >> I'd say number 2 is best. Rationale: grep :)
> >> I agree it's visually somewhat harder, but grepping is way easier.
> >
> > I don't see how (2) is any tougher to grep than (3)...?
[...]
> > $ grep 'fstat.*fd.*statbuf' man2/*
>
> There are a few cases: if I want to find declarations of type int,
> I'd start with:
>
> $ grep -rn "int\s"
>
> or something like that. "int\~" would break the ability to do that.
That would, among more obscure cases, miss the style of function
declaration used by people who get along without ctags:
static int
my_little_function(int foo, char bar)
So I would tend to use grep 'int\>' to match a word boundary instead of
a whitespace character.
Regards,
Branden
From: Alejandro Colomar
> Sent: 28 September 2020 14:33
...
> There are a few cases: if I want to find declarations of type int,
> I'd start with:
>
> $ grep -rn "int\s"
>
> or something like that. "int\~" would break the ability to do that.
The 'word markers' \< and \> are your friends; look for "\<int\>".
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On 2020-09-28 15:48, G. Branden Robinson wrote:
>> There are a few cases: if I want to find declarations of type int,
>> I'd start with:
>>
>> $ grep -rn "int\s"
>>
>> or something like that. "int\~" would break the ability to do that.
>
> That would, among more obscure cases, miss the style of function
> declaration used by people who get along without ctags:
>
> static int
> my_little_function(int foo, char bar)
>
> So I would tend to use grep 'int\>' to match a word boundary instead of
> a whitespace character.
>
> Regards,
> Branden
>
On 2020-09-28 16:31, David Laight wrote:
> From: Alejandro Colomar
>> Sent: 28 September 2020 14:33
> ...
>> There are a few cases: if I want to find declarations of type int,
>> I'd start with:
>>
>> $ grep -rn "int\s"
>>
>> or something like that. "int\~" would break the ability to do that.
>
> The 'word markers' \< and \> are your friends; look for "\<int\>".
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
Thank you both, I didn't know about those.
Regards,
Alex
Hi Branden,
On 9/27/20 7:46 AM, G. Branden Robinson wrote:
> At 2020-09-24T10:06:23+0200, Michael Kerrisk (man-pages) wrote:
>> Thanks for the interesting history, Branden!
>
> Hi, Michael. And you're welcome! I often wonder if I test people's
> patience with my info dumps but I try to show my work when making
> claims.
>
>> From time toi time I wonder if the function prototypes in
>> the SYNOPSIS should also be inside .EX/.EE. Your thoughts?
>
> I think there are trade-offs.
>
> 1. If you want alignment, the monospaced font that .EX/.EE uses is the
> most portable way to get it.
> 2. For typeset output, you'll generally run out of line more quickly
> with a monospaced font than with the troff/man default (Times).
> _Any_ time filling is off, output should be checked to see if it
> overruns the right margin, but this point strengthens in monospace.
Yes, it's a good point. I think I'll leave this idea for now.
> Here's something that isn't a trade-off that might come as a surprise to
> some readers.
>
> * You can still get bold and italics inside an .EX/.EE region, so you
> can still use these distinguish data types, variable names, and
> what-have-you.
>
> The idiom for achieving this is apparently not well-known among those
> who write man pages by hand, and tools that generate man(7) language
> from some other source often produce output that is so ugly as to be
> unintelligible to non-experts in *roff.
>
> The key insights are that (A) you can still make macro calls inside an
> .EX/.EE region, and (B) you can use the \c escape to "interrupt" an
> input line and continue it on the next without introducing any
> whitespace. For instance, looking at fstat() from your stat(2) page, I
> might write it using .EX and .EE as follows:
>
> .EX
> .B int fstat(int \c
> .IB fd , \~\c
> .B struct stat *\c
> .IB statbuf );
> .EE
>
> Normally in man pages, it is senseless to have any spaces before the \c
> escape, and \c is best omitted in that event. However, when filling is
> disabled (as is the case in .EX/.EE regions), output lines break where
> the input lines do by default--\c overrides this, causing the lines to
> be joined. (Regarding the \~, see below.)
>
> If there is no use for roman in the line, then you could do the whole
> function signature with the .BI macro by quoting macro arguments that
> contain whitespace.
I was more or less aware of all of the above. (But the \c technique
is something that I see rarely enough that I often take a moment to
remember what it does.)
>
> .EX
> .BI "int fstat(int " fd ", struct stat *" statbuf );
> .EE
>
> As a matter of personal style, I find quoted space characters interior
> but adjacent to quotation marks visually confusing--it's slower for me
> to tell which parts of the line are "inside" the quotes and which
> outside--so I turn to groff's \~ non-breaking space escape (widely
> supported elsewhere) for these boundary spaces.
>
> .EX
> .BI "int fstat(int\~" fd ", struct stat *" statbuf );
> .EE
>
> Which of the above three models do you think would work best for the
> man-pages project?
I understand what you say about quoted interior spaces being
a little hard to parse. But I find the \~ makes the source
less readable. Likewise, IMO, the \c technique renders the source
harder to read.
> Also, do you have any use for roman in function signatures? I see it
> used for comments and feature test macro material, but not within
> function signatures proper.
I think you're correct. Roman only occurs in comments.
>
> As an aside, I will admit to some unease with the heavy use of bold in
> synopses in section 2 and 3 man pages,
It's been that way "forever" in the Linux man-pages.
> but I can marshal no historical
> argument against it. In fact, a quick check of some Unix v7 section 2
> pages from 1979 that I have lying around (thanks to TUHS) reveals that
> Bell Labs used undifferentiated bold for the whole synopsis!
>
> $ head -n 13 usr/man/man2/stat.2
> .TH STAT 2
> .SH NAME
> stat, fstat \- get file status
> .SH SYNOPSIS
> .B #include <sys/types.h>
> .br
> .B #include <sys/stat.h>
> .PP
> .B stat(name, buf)
> .br
> .B char *name;
> .br
> .B struct stat *buf;
As ever, thanks for the history!
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On 9/27/20 10:05 PM, Alejandro Colomar wrote:
> Hi Branden,
>
> * G. Branden Robinson via linux-man:
>
> 1)
>
> > .EX
> > .B int fstat(int \c
> > .IB fd , \~\c
> > .B struct stat *\c
> > .IB statbuf );
> > .EE
>
> 2)
>
> > .EX
> > .BI "int fstat(int " fd ", struct stat *" statbuf );
> > .EE
>
> 3)
>
> > .EX
> > .BI "int fstat(int\~" fd ", struct stat *" statbuf );
> > .EE
>
> I'd say number 2 is best. Rationale: grep :)
> I agree it's visually somewhat harder, but grepping is way easier.
I'd say number 2 also. But, visually, it's the least difficult
for me.
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
> 2.- Use sizeof() everywhere, and the macro for the initializer.
>
> pros:
> - It is valid as long as the buffer is an array.
> cons:
> - If the code gets into a function, and the buffer is then a pointer,
> it will definitively produce a silent bug.
Sigh! I just did exactly the last point in a test program I've been writing...
M
On 2020-09-29 15:38, Michael Kerrisk (man-pages) wrote:
>> 2.- Use sizeof() everywhere, and the macro for the initializer.
>>
>> pros:
>> - It is valid as long as the buffer is an array.
>> cons:
>> - If the code gets into a function, and the buffer is then a pointer,
>> it will definitively produce a silent bug.
>
> Sigh! I just did exactly the last point in a test program I've been writing...
>
> M
>
Hmmm, maybe you would like to comment on this LKML thread I started
yesterday:
https://lore.kernel.org/lkml/[email protected]
Concretely, point 4 is about this.
I'd push 'array_bytes()' to all "libc"s I can, so that it's then common
enough to use it everywhere, even in the man.
I'd also recommend reading this StackOverflow answer I wrote last year:
https://stackoverflow.com/a/57537491/6872717
Cheers,
Alex