lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 24 Dec 2020 14:39:16 -0800 From: Joe Perches <joe@...ches.com> To: Tom Rix <trix@...hat.com>, Simon Horman <simon.horman@...ronome.com> Cc: kuba@...nel.org, davem@...emloft.net, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, kafai@...com, songliubraving@...com, yhs@...com, john.fastabend@...il.com, kpsingh@...nel.org, gustavoars@...nel.org, louis.peens@...ronome.com, netdev@...r.kernel.org, bpf@...r.kernel.org, oss-drivers@...ronome.com, linux-kernel@...r.kernel.org Subject: Re: [PATCH] nfp: remove h from printk format specifier On Thu, 2020-12-24 at 14:14 -0800, Tom Rix wrote: > On 12/24/20 12:21 PM, Simon Horman wrote: > > On Wed, Dec 23, 2020 at 12:20:53PM -0800, trix@...hat.com wrote: > > > From: Tom Rix <trix@...hat.com> > > > > > > This change fixes the checkpatch warning described in this commit > > > commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of unnecessary %h[xudi] and %hh[xudi]") > > > > > > Standard integer promotion is already done and %hx and %hhx is useless > > > so do not encourage the use of %hh[xudi] or %h[xudi]. > > > > > > Signed-off-by: Tom Rix <trix@...hat.com> > > Hi Tom, > > > > This patch looks appropriate for net-next, which is currently closed. > > > > The changes look fine, but I'm curious to know if its intentionally that > > the following was left alone in ethernet/netronome/nfp/nfp_net_ethtool.c:nfp_net_get_nspinfo() > > > > snprintf(version, ETHTOOL_FWVERS_LEN, "%hu.%hu" > > I am limiting changes to logging functions, what is roughly in checkpatch. > > I can add this snprintf in if you want. I'm a bit confused here Tom. I thought your clang-tidy script was looking for anything marked with __printf() that is using %h[idux] or %hh[idux]. Wouldn't snprintf qualify for this already? include/linux/kernel.h-extern __printf(3, 4) include/linux/kernel.h:int snprintf(char *buf, size_t size, const char *fmt, ...); Kernel code doesn't use a signed char or short with %hx or %hu very often but in case you didn't already know, any signed char/short emitted with anything like %hx or %hu needs to be left alone as sign extension occurs so: signed char foo = -1; printk("%hx", foo); emits ffff but printk("%x", foo); emits ffffffff An example: $ gcc -x c - #include <stdio.h> #include <stdlib.h> int main(int argc, char **argv) { signed short i = -1; printf("hx: %hx\n", i); printf("x: %x\n", i); printf("hu: %hu\n", i); printf("u: %u\n", i); return 0; } $ ./a.out hx: ffff x: ffffffff hu: 65535 u: 4294967295 $
Powered by blists - more mailing lists