svn commit: r273576 - head/sys/dev/uart

[Marcelo Araujo]

2014-10-24 13:58 GMT+08:00 Mateusz Guzik <mjguzik at gmail.com>:

> On Fri, Oct 24, 2014 at 05:39:32AM +0000, Marcelo Araujo wrote:
> > Author: araujo (ports committer)
> > Date: Fri Oct 24 05:39:32 2014
> > New Revision: 273576
> > URL: https://svnweb.freebsd.org/changeset/base/273576
> >
> > Log:
> >   Fix a leaked Storage Variable.
> >
> >  int
> >  uart_getenv(int devtype, struct uart_devinfo *di, struct uart_class
> *class)
> >  {
> > -     const char *spec;
> > +     const char *cp, *spec;
> >       bus_addr_t addr = ~0U;
> >       int error;
> >
> > @@ -214,12 +214,12 @@ uart_getenv(int devtype, struct uart_dev
> >        * port (resp).
> >        */
> >       if (devtype == UART_DEV_CONSOLE)
> > -             spec = kern_getenv("hw.uart.console");
> > +             cp = kern_getenv("hw.uart.console");
> >       else if (devtype == UART_DEV_DBGPORT)
> > -             spec = kern_getenv("hw.uart.dbgport");
> > +             cp = kern_getenv("hw.uart.dbgport");
> >       else
> > -             spec = NULL;
> > -     if (spec == NULL)
> > +             cp = NULL;
> > +     if (cp == NULL)
> >               return (ENXIO);
> [..]
> >               default:
> > +                     freeenv(__DECONST(char *, cp));
> >                       return (EINVAL);
> >               }
> >               if (*spec == '\0')
> >                       break;
> > -             if (*spec != ',')
> > +             if (*spec != ',') {
> > +                     freeenv(__DECONST(char *, cp));
> >                       return (EINVAL);
> > +             }
> >               spec++;
> >       }
> > +     freeenv(__DECONST(char *, cp));
> >
>
> Why not 'char *cp;'? That would avoid __DECONST entirely and would not
> require spec to change type.
>

Well, it might be possible to use 'char *cp', however as I'm not aware of
all uart implementation, I just followed the previous 'spec' declaration
type that is a constant too. I'm gonna take a look on it, to check if there
is any backward to only use 'char *cp'.


>
> There are some cosmetics around that may be worth fixing (e.g. if,
> else-if instead of switch, while (1) instead of for (;;)).
>

Yes, that for sure makes sense and it is worth.

Thanks to point all of these things.
Best Regards,
-- 

-- 
Marcelo Araujo            (__)araujo at FreeBSD.org
\\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>   \/  \ ^
Power To Server.         .\. /_)