linux-kernel - Re: [PATCHv2] vsprintf: Add %pMR for Bluetooth MAC address

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: <20120528102055.GA3537@aemeltch-MOBL1>
Date:	Mon, 28 May 2012 13:20:57 +0300
From:	Andrei Emeltchenko <Andrei.Emeltchenko.news@...il.com>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	linux-bluetooth@...r.kernel.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, adobriyan@...il.com,
	andriy.shevchenko@...ux.intel.com
Subject: Re: [PATCHv2] vsprintf: Add %pMR for Bluetooth MAC address

Hi Andy,

On Mon, May 28, 2012 at 01:01:05PM +0300, Andy Shevchenko wrote:
> On Mon, May 28, 2012 at 12:00 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@...il.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@...el.com>
> >
> > Bluetooth uses mostly LE byte order which is reversed for visual
> > interpretation. Currently in Bluetooth in use unsafe batostr function.
> >
> > This is slightly modified version of Joe Perches <joe@...ches.com>
> > patch (sent Sat, Dec 4, 2010).
> >
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@...el.com>
> > ---
> >        v2: changed bluetooth to reversed, syntax fixes
> >
> >  lib/vsprintf.c |   22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index abbabec..d98b12d 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -557,17 +557,27 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
> >  {
> >        char mac_addr[sizeof("xx:xx:xx:xx:xx:xx")];
> >        char *p = mac_addr;
> > -       int i;
> > +       int i, index;
> >        char separator;
> > +       bool reversed = false;
> >
> > -       if (fmt[1] == 'F') {            /* FDDI canonical format */
> > +       switch (fmt[1]) {
> > +       case 'F':
> >                separator = '-';
> > -       } else {
> > +               break;
> > +
> > +       case 'R':
> > +               reversed = true;
> > +               /* fall through */
> This solution looks a bit limited. On one hand it makes difficult to add another
> case where format specifies colon separator with something else. On
> the other hand

I believe it is good as is for now. The other option would be to name it
as "B" or "b" for bluetooth.

> I don't see any troubles if you allow reverse as a modifier for both
> cases %pMF & %pM

MF is used for FDDI and it makes no sense to reverse it.

> 
> > +
> > +       default:
> >                separator = ':';
> > +               break;
> >        }
> >
> >        for (i = 0; i < 6; i++) {
> > -               p = hex_byte_pack(p, addr[i]);
> > +               index = !reversed ? i : 5 - i;
> > +               p = hex_byte_pack(p, addr[index]);
> I guess instead of using additional variable (index), you could use
> just normal if () {} else {} sentence
> here.

I can change this

Best regards 
Andrei Emeltchenko 
--
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/