From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 290FA187346 for ; Fri, 16 Jan 2026 09:32:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768555926; cv=none; b=EiOogaXujmdvhSdloXw4caGvXUXlF1QTQMyav5W3tBB8zbTAp6z51qjT8GVOugZZ8cwk0dysryVIfSrKIUtgojAVGB7Qiz4l22kx8STr6xwqiznkq8Vjk3sfIaK8sOm6kRZ7+rVba3UAeR0nB1ppj+fEElvAMoZgIRSMlJFeHjU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768555926; c=relaxed/simple; bh=ZdFXc8gSvisnmGw5+nADR9O8j0u4f9hcomPajV+ma8A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OdlAXBzp4VSxblpze5gHs4EouZ1k2wIys2oDhhydccZiS5m5GVPr/jmp8/1xs/KbFuC7yQJY6LXp8QeiNS5HxfHZTS6432nX5fckfx2kS3u3PY41kjVbGOk83HDNGER4uIleMg0XILCXCc4VBZ/m6+OVuW7PMjRiMDjTUWq/a70= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=UmdDSiQy; arc=none smtp.client-ip=209.85.221.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="UmdDSiQy" Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-42fed090e5fso952079f8f.1 for ; Fri, 16 Jan 2026 01:32:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1768555922; x=1769160722; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=WbidHmVUKVZZtajdeoRk2JPxwzQ38apdVGqLw2XUjQU=; b=UmdDSiQyxj7tHv7vx+qnCRUBT2X04/y/DdrpiNirg7Uf8mAjiyhoW2cRzvIIhNDCJO e59z9LZONry1+zJ2JnqjNJDYMzmkCzw3rQ02NknKkiSmMnp5fTsaZ6ZfLaJeNtfuivyv 6DvIYxEhfSVtnrYG1oOPXaV3aB+dJCRa0sGKoYPQ/K4ZEdW+Qsq6D3W52dFQ651aPhzO gHsdx0ezkW5ih75fI6Qy99c4QiA3kknMWyYxCJZVpBfU+0VNQ7u2RzFyFATz/+jmj8Ym vWfDarDgc9ZFZtKgv8vqpcD0rjtL89pnaCU+epTxcLg9pekEHJt6DvlGDZepopbyG+/h Rocg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768555922; x=1769160722; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=WbidHmVUKVZZtajdeoRk2JPxwzQ38apdVGqLw2XUjQU=; b=iv0xJQ5sgZvkNKakcJmS+D+ZLaVnDwOEewTZL3Q1S14O7h8jMl4SVSh5+6HOB3Dnbn LWBkkRKv9rVYElYQ/P6gbI8MWnFXlTxww/f6peCzJPDcKBOakpDB1CDebloJz/3ZdrGr maKi9kYQon+UfK16hfPo4X7+UD9R8CGzlU11aex2IWWXqpODHiJ9eaapqmYIRBkVGjfS UTQFCTA/l96CQPA+uB5tGWVStifY08flsn4KiATDYRBci42PIkCY/1tG4xuTAjDiQqL5 Tb6Cnws15PIrRkebX4Y2aa/Nbdg/pCWj80m6hutkcEkzhwL7LUdLCa1ESDfW4f0vlFKG WYsQ== X-Forwarded-Encrypted: i=1; AJvYcCXAqfCle5FZODQHj6mM6xBfKrojNTcgmxCLFkg38MGFqZdUhoWA+v4Vo/IypslOwGbQggDClHJ9F198p7g=@vger.kernel.org X-Gm-Message-State: AOJu0YzU+N7+0EbCQntauSMx21F5mDgtEORpMxiXiBHv0emesKpWCbAY rT578RSS/gUu9CnGozYEBG3iwlF0ZNUnng/VZ7qxnQtJGWrTxc+TGJWLHWfkbvkaPog= X-Gm-Gg: AY/fxX5pIlwXKvcZaljcLOuZf94J6weuOmAr4vkQ0uam61+BXVND+IvXhKbY9skTS/i asAiJpIF0Xcw9TaBE8IjoE2SVuN2gIxJ7lgT0Yx/CarXDi0R2440IYZbJ/IWiMgSUt+0phejRqo 1Yxe+sFwfUpLG2Xb/Dxdhe5V+rarS5VY3UDurPBXF7FUxCQOGA4ayFDnsN96YETvYLJdOnzmk5U MsZWFGaf1R29MysuKBzhYtl0EdNVVqbBE8SuuMTv5hqJjf8aspCD9e4w47tURX+UOhhfNJt8jst GUv1a+EEdMWAnw1dnGU/FxmBqspOsv0CrKSxdJaSAUogQGXQOwjhG+k9Nk39VymW8njSdiId1V+ OWB4IMVgE7fxIUYRAJ135yl0diwu98UcxELQb9kyfKYzr4I+g4s8dSF/fX7/53Wa0mVtYW5Juz5 N8CinCK+Y969JhZQ== X-Received: by 2002:a05:6000:2910:b0:42f:b0ab:7b48 with SMTP id ffacd0b85a97d-43569979c52mr2495306f8f.1.1768555922343; Fri, 16 Jan 2026 01:32:02 -0800 (PST) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4356996dad0sm4043723f8f.27.2026.01.16.01.32.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Jan 2026 01:32:01 -0800 (PST) Date: Fri, 16 Jan 2026 10:31:58 +0100 From: Petr Mladek To: Tamir Duberstein Cc: Andy Shevchenko , Steven Rostedt , Rasmus Villemoes , Sergey Senozhatsky , Kees Cook , linux-kernel@vger.kernel.org, oe-kbuild-all@lists.linux.dev, kernel test robot Subject: Re: [PATCH] printf: add __printf attribute Message-ID: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu 2026-01-15 09:53:37, Tamir Duberstein wrote: > On Tue, Dec 16, 2025 at 5:13 AM Petr Mladek wrote: > > > > On Mon 2025-12-08 16:07:28, Tamir Duberstein wrote: > > > On Mon, Dec 8, 2025 at 9:06 AM Petr Mladek wrote: > > > > > > > > On Mon 2025-12-08 15:30:53, Andy Shevchenko wrote: > > > > > On Sun, Dec 07, 2025 at 08:32:53PM -0500, Tamir Duberstein wrote: > > > > > > On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko > > > > > > wrote: > > > > > > > On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote: > > > > > > > > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko > > > > > > > > wrote: > > > > > > > > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote: > > > > > > > > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko > > > > > > > > > > wrote: > > > > > > > > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote: > > > > > > > > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko > > > > > > > > > > > > wrote: > > > > > > > > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > -static void > > > > > > > > > > > > > > +static void __printf(2, 3) > > > > > > > > > > > > > > > > > > > > > > > > > > 3?! > > > > > > > > > > > > > > > > > > > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > > > > > > > > > > > > > string, but the second parameter tells compiler to check the variadic > > > > > > > > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align > > > > > > > > > > > > > it with the given __printf() attribute, but I don't know if this what we want. > > > > > > > > > > > > > > > > > > > > > > > > The second parameter is the first-to-check, it is not specific to > > > > > > > > > > > > variadic arguments. Using 0 means that no arguments are checkable, so > > > > > > > > > > > > the compiler only validates the format string itself and won’t > > > > > > > > > > > > diagnose mismatches with `p`. This works whether or not we later > > > > > > > > > > > > change `const void *p` to `...`. > > > > > > > > > > > > > > > > > > > > > > Yes, but this is fragile. As I explained it works only because we supply > > > > > > > > > > > the format string stuck to "%p", anything else will require reconsidering > > > > > > > > > > > the function prototypes. So, strictly speaking this should be (2, 0) if > > > > > > > > > > > we leave const void *p as is. > > > > > > > > > > > > > > > > > > > > > I believe this is not correct. As I said, 0 means "do not check > > > > > > > > > > arguments" so only the format string will be checked. See the existing > > > > > > > > > > uses of this annotation in this file: > > > > > > > > > > > > > > > > > > > > static void __printf(7, 0) > > > > > > > > > > do_test(struct kunit *kunittest, const char *file, const int line, int > > > > > > > > > > bufsize, const char *expect, > > > > > > > > > > int elen, const char *fmt, va_list ap) > > > > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > static void __printf(6, 7) > > > > > > > > > > __test(struct kunit *kunittest, const char *file, const int line, > > > > > > > > > > const char *expect, int elen, > > > > > > > > > > const char *fmt, ...) > > > > > > > > > > > > > Since it doesn't make much sense to make this function variadic, I > > > > > > think the best we can do is a macro wrapper that combines this > > > > > > function with `no_printk`. Something like > > > > > > > > > > > > #define test_hashed(kunittest, fmt, p) \ > > > > > > do { \ > > > > > > if (0) \ > > > > > > no_printk(fmt, p); \ > > > > > > __test_hashed(kunittest, fmt, p);\ > > > > > > } while (0) > > > > > > > > > > > > > IMHO, this is is not worth it. test_hashed(kunittest, fmt, p) calls > > > > test(buf, fmt, p). It goes down to __test() which does the format > > > > check: > > > > > > > > static void __printf(6, 7) > > > > __test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen, > > > > const char *fmt, ...) > > > > > > > > > > > > > > That would give us better diagnostics, but is more complex (and more > > > > > > lines of code than just repeating this function's body twice, which > > > > > > would also give good diagnostics). I think the best thing to do is just > > > > > > to ignore the report that prompted me to look into this. Please let me > > > > > > know if you disagree. > > > > > > > > > > I think we may not ignore the report as it breaks builds in some cases. > > > > > As I said > > > > > > > > > > - __printf(2, 0) for now > > > > > > > > > > - and perhaps a comment on top to explain the clang approach that may cope > > > > > with fixed-argument functions for -Wformat (you can even put a link to that > > > > > LLVM discussion about the feature). > > > > > > > > I personally prefer this way. We just need to calm down the warning. > > > > The proper check is done by the nested test()... > > > > > > The nested `__test()` call cannot do the proper check because it cannot > > > see the format string. Right? > > > > IMHO, it does see the format string. It is defined the following way: > > > > static void __printf(6, 7) > > __test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen, > > const char *fmt, ...) > > > > #define test(expect, fmt, ...) \ > > __test(kunittest, __FILE__, __LINE__, expect, strlen(expect), fmt, ##__VA_ARGS__) > > > > static void > > test_hashed(struct kunit *kunittest, const char *fmt, const void *p) > > { > > char buf[PLAIN_BUF_SIZE]; > > > > plain_hash_to_buffer(kunittest, p, buf, PLAIN_BUF_SIZE); > > > > test(buf, fmt, p); > > } > > > > > > Now, let's get for example: > > > > test_hashed(kunittest, "%p", PTR_INVALID); > > > > it calls: > > > > test(buf, "%p", PTR_INVALID); > > > > which is exapnded to: > > > > __test(kunittest, file, line, buf, strlen(buf), "%p", PTR_INVALID) > > > > so, it gets the same printf arguments as the original test_hashed, > > namely: > > > > %p, PTR_INVALID > > > > Or do I miss anything, please? > > the problem is that test_hashed is a function, not a macro, so it is > not correct to say that > > test_hashed(kunittest, "%p", PTR_INVALID) > > is expanded to > > __test(kunittest, file, line, buf, strlen(buf), "%p", PTR_INVALID) > > thus the compiler cannot perform any meaningful checking of the > test_hashed call. > > We could make test_hashed a macro, though. I am sorry but I still think that it is not worth it. The proposed changes add more complexity or weird stuff for a little gain. > > You might argue that it works by chance and that it might change in the > > future. But I have hard times to imagine it. test_hashed() is just > > a wrapper around "test()". The only purpose is to fill "buf" with > > the expected outcome. > > > > If any refactoring is needed in the future. The __printf() macros > > would need some refactoring as well. IMHO, the above is still valid. > Apologies for taking a month to reply here. No problem at all. Best Regards, Petr