On Sat, 10 Dec 2022 00:12:58 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > On 12/9/22 12:05 PM, Xin Liu wrote: > > This is a small improvement in libbpf_strerror. When libbpf_strerror > > is used to obtain the system error description, if the length of the > > buf is insufficient, libbpf_sterror returns ERANGE and sets errno to > > ERANGE. > > > > However, this processing is not performed when the error code > > customized by libbpf is obtained. Make some minor improvements here, > > return -ERANGE and set errno to ERANGE when buf is not enough for > > custom description. > > nit: $subject line got corrupted? > > > Signed-off-by: Xin Liu <liuxin350@xxxxxxxxxx> > > --- > > > > v2: > > Check the return value of snprintf to determine whether the buffer is > > too small. > > > > v1: > > https://lore.kernel.org/bpf/20221209084047.229525-1-liuxin350@xxxxxxxxxx/T/#t > > > > tools/lib/bpf/libbpf_errno.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c > > index 96f67a772a1b..6240c7cb7472 100644 > > --- a/tools/lib/bpf/libbpf_errno.c > > +++ b/tools/lib/bpf/libbpf_errno.c > > @@ -39,14 +39,13 @@ static const char *libbpf_strerror_table[NR_ERRNO] = { > > > > int libbpf_strerror(int err, char *buf, size_t size) > > { > > + int ret; > > nit: newline after declaration > > > if (!buf || !size) > > return libbpf_err(-EINVAL); > > > > err = err > 0 ? err : -err; > > > > if (err < __LIBBPF_ERRNO__START) { > > - int ret; > > - > > ret = strerror_r(err, buf, size); > > buf[size - 1] = '\0'; > > return libbpf_err_errno(ret); > > @@ -56,12 +55,20 @@ int libbpf_strerror(int err, char *buf, size_t size) > > const char *msg; > > > > msg = libbpf_strerror_table[ERRNO_OFFSET(err)]; > > - snprintf(buf, size, "%s", msg); > > + ret = snprintf(buf, size, "%s", msg); > > buf[size - 1] = '\0'; > > + if (ret < 0) > > + return libbpf_err_errno(ret); > > This would pass in ret == -1 and then eventually return 1 which > is misleading, no? > > We have buf and msg non-NULL and a positive size, afaik, the only > case where you could get a negative error now is when you pass in > a buf with size exceeding INT_MAX.. > > > + if (ret >= size) > > + return libbpf_err(-ERANGE); > > return 0; > > } > > > > - snprintf(buf, size, "Unknown libbpf error %d", err); > > + ret = snprintf(buf, size, "Unknown libbpf error %d", err); > > buf[size - 1] = '\0'; > > + if (ret < 0) > > + return libbpf_err_errno(ret); > > + if (ret >= size) > > + return libbpf_err(-ERANGE); > > return libbpf_err(-ENOENT); > > } > > The logic of returning negative numbers is really unlikely here, I'll add some comments and delete this logic, thanks to Daniel for pointing out.
