On 11/24/15 18:38, Eric Blake wrote: > On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: >> On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: > >>> >>> drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects argument of type 'long long int *', but argument 3 has type 'phys_addr_t *' [-Wformat=] >>> &ctrl_off, &data_off, &consumed); >>> ^ >> >> Oh, I think I know why this happened: >> > >> >> So, I could use u64 instead of phys_addr_t and resource_size_t, and >> keep "%lli" (or "%Li"), but then I'd have to check if the parsed value > > %Li is not POSIX. Don't use it (stick with %lli). > >> would overflow a 32-bit address value on arches where phys_addr_t is >> u32, which would make things a bit more messy and awkward. >> >> I'm planning on #ifdef-ing the format string instead: >> >> #ifdef CONFIG_PHYS_ADDR_T_64BIT >> #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" >> #else >> #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" >> #endif > > A more typical approach is akin to <inttypes.h>; have PH_ADDR_FMT > defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT > "%n:..., ...), using PH_ADDR_FMT multiple times. > >> ... >> processed = sscanf(str, PH_ADDR_SCAN_FMT, >> &base, &consumed, >> &ctrl_off, &data_off, &consumed); > > Umm, why are you passing &consumed to more than one sscanf() %? That's > (probably) undefined behavior. > > [In general, sscanf() is a horrid interface to use for parsing integers > - it has undefined behavior if the input text would trigger integer > overflow, making it safe to use ONLY on text that you control and can > guarantee won't overflow. By the time you've figured out if untrusted > text meets the requirement for safe parsing via sscanf(), you've > practically already parsed it via safer strtol() and friends.] > Yes, but this is the kernel, which may or may not follow POSIX semantics. (And may or may not curse at POSIX in the process, either way! :)) Laszlo -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
