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

[Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>]

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@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.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@perches.com>
> > patch (sent Sat, Dec 4, 2010).
> >
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.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