On 2018-04-09 20:16, jeffm@xxxxxxxx wrote:
> From: Jeff Mahoney <jeffm@xxxxxxxx>
>
> This patch cleans up the reiserfs print formatting code by getting
> rid of the do_reiserfs_warning macro. We push the va_list handling
> and error_buf locking into the caller, which makes prepare_error_buf
> more like a snprintf routine, so we'll call it that: reiserfs_snprintf.
>
>
> +static DEFINE_SPINLOCK(error_lock);
> +static char error_buf[2048];
> +
> void __reiserfs_warning(struct super_block *sb, const char *id,
> const char *function, const char *fmt, ...)
> {
> - do_reiserfs_warning(fmt);
> + va_list args;
> +
> + va_start(args, fmt);
> +
> + spin_lock(&error_lock);
> + reiserfs_snprintf(error_buf, sizeof(error_buf) - 1, fmt, &args);
> + spin_unlock(&error_lock);
> +
Subtracting 1 from the sizeof() is... unusual. Why? snprintf itself
wants to know the size of the buffer it is printing to, and unless I'm
misreading your reiserfs_snprintf, that's also what that (and all the
functions it calls) wants to know.
What is the point of introducing the separate fmt_lock spinlock if it
will only ever be taken while already holding error_lock?
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
