linux-kernel - Re: [PATCH 0/3] jfs: logging neatening

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1459354939.25110.123.camel@perches.com>
Date:	Wed, 30 Mar 2016 09:22:19 -0700
From:	Joe Perches <joe@...ches.com>
To:	Dave Kleikamp <dave.kleikamp@...cle.com>,
	JFS Discussion <jfs-discussion@...ts.sourceforge.net>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] jfs: logging neatening

On Wed, 2016-03-30 at 10:56 -0500, Dave Kleikamp wrote:
> On 03/30/2016 07:23 AM, Joe Perches wrote:
> > 
> > This patchset fixes the uses of jfs_info, jfs_warn and jfs_err that
> > have terminating newlines and a couple other trivialities to make
> > the logging a bit more consistent.
> These patches look good. I'm pushing them out to the -next build.
> 
> > 
> > There is a difference in use between jfs_error and the other
> > jfs_info, jfs_warn, and jfs_err logging macros.  jfs_error is more
> > like the rest of the kernel and requires a newline as the last
> > character of the format.
> > 
> > The jfs_info, jfs_warn, and jfs_err macros add the terminating
> > newline to the format so the uses do not require them.
> I think there's an argument for both ways of doing it. I'm sure I had my
> reasons for automatically adding the newline back when I implemented
> those macros. (They probably should be inline functions, but that's
> another issue.)

Nah.  It was me.  I changed jfs_error awhile back to move the
newline to the uses.

commit eb8630d7d2fd13589e6a7a3ae2fe1f75f867fbed
Author: Joe Perches <joe@...ches.com>
Date:   Tue Jun 4 16:39:15 2013 -0700

    jfs: Update jfs_error
    
    Use a more current logging style.
    
    Add __printf format and argument verification.
    
    Remove embedded function names from formats.
    Add %pf, __builtin_return_address(0) to jfs_error.
    Add newlines to formats for kernel style consistency.
    (One format already had an erroneous newline)
    Coalesce formats and align arguments.
    
    Object size reduced ~1KiB.
    
    $ size fs/jfs/built-in.o*
       text        data     bss     dec     hex filename
     201891       35488   63936  301315   49903 fs/jfs/built-in.o.new
     202821       35488   64192  302501   49da5 fs/jfs/built-in.o.old

Using inline functions would actually be more code as
you'd have to handle the log level and newline via
a vprintk of some type.  At least the test could be
consolidated into the inline though.

Many of the jfs_info calls appear to be function
tracing and perhaps could be eliminated altogether.

> It might be better if the jfs_info, jfs_warn, and jfs_err macros
> > were changed to require a newline termination and the uses changed
> > to include the newline, but that's a larger change.
> Yeah, these patches are the obvious improvement, without changing
> anything from a design standpoint.