linux-kernel - Re: [patch 2/3] Add a WARN() macro; this is WARN

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19f34abd0805062341x21caa29dld03d838057d498e3@mail.gmail.com>
Date:	Wed, 7 May 2008 08:41:31 +0200
From:	"Vegard Nossum" <vegard.nossum@...il.com>
To:	"Arjan van de Ven" <arjan@...radead.org>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: [patch 2/3] Add a WARN() macro; this is WARN_ON() + printk arguments

Hi!

On Wed, May 7, 2008 at 8:21 AM, Arjan van de Ven <arjan@...radead.org> wrote:
> Subject: Add a WARN() macro; this is WARN_ON() + printk arguments
>  From: Arjan van de Ven <arjan@...ux.intel.com>
>
>  Add a WARN() macro that acts like WARN_ON(), with the added feature that
>  it takes a printk like argument that is printed as part of the warning
>  message.

[...]

>  +#ifndef WARN
>  +#define WARN(condition, format...) ({                                          \
>  +       int __ret_warn_on = !!(condition);                              \
>  +       if (unlikely(__ret_warn_on))                                    \
>  +               __WARN_printf(format);                                  \
>  +       unlikely(__ret_warn_on);                                        \
>  +})
>  +#endif

Is there a good reason why this is not a static inline function? If
I've understood correctly, we want to turn as many macros as possible
into functions, and I don't see an immediate reason why this one can't
be one.

>  +void warn_slowpath(const char *file, int line, const char *fmt, ...)
>  +{
>  +       va_list args;
>  +
>  +
>  +       char function[KSYM_SYMBOL_LEN];
>  +       unsigned long caller = (unsigned long) __builtin_return_address(0);

If WARN() is made a static inline, you can call
__builtin_return_address(0) there and pass it into here instead. This
seems like a kind of low-level internal function anyway, because of
the file/line info.

OTOH, why can't you use __FUNCTION__ or __func__ to determine the
caller (in WARN) rather than doing it here, at run-time? If it's to
save space (or something like that), I think it should be documented?

>  +       sprint_symbol(function, caller);
>  +
>  +       printk(KERN_WARNING "------------[ cut here ]------------\n");
>  +       printk(KERN_WARNING "WARNING: at %s:%d %s()\n", file,
>  +               line, function);
>  +       va_start(args, fmt);
>  +       vprintk(fmt, args);
>  +       va_end(args);
>  +
>  +       print_modules();
>  +       dump_stack();
>  +       print_oops_end_marker();
>  +       add_taint(TAINT_WARN);
>  +}
>  +EXPORT_SYMBOL(warn_slowpath);
>   #endif


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/