[PATCH libBSD] iicbus/rtems-i2c.c: Add rtems, path as an additional bus path

[Christian Mauderer]

Hello Niteesh,

On 19/04/2021 18:45, Niteesh G. S. wrote:
> 
> 
> On Mon, Apr 19, 2021 at 9:55 PM Gedare Bloom <gedare at rtems.org 
> <mailto:gedare at rtems.org>> wrote:
> 
>     On Sun, Apr 18, 2021 at 9:12 AM G S Niteesh Babu
>     <niteesh.gs at gmail.com <mailto:niteesh.gs at gmail.com>> wrote:
>      >
>      > Adds "rtems,path" as an additional bus path for the i2c driver.
>      > Previously the bus path was provided in "rtems,i2c-path" property
>      > only.
>      > ---
>      >  rtemsbsd/sys/dev/iicbus/rtems-i2c.c | 9 ++++++---
>      >  1 file changed, 6 insertions(+), 3 deletions(-)
>      >
>      > diff --git a/rtemsbsd/sys/dev/iicbus/rtems-i2c.c
>     b/rtemsbsd/sys/dev/iicbus/rtems-i2c.c
>      > index b965f248..7ac7a7c4 100644
>      > --- a/rtemsbsd/sys/dev/iicbus/rtems-i2c.c
>      > +++ b/rtemsbsd/sys/dev/iicbus/rtems-i2c.c
>      > @@ -87,9 +87,12 @@ rtems_i2c_attach(device_t dev)
>      >
>      >         len = OF_getprop_alloc(node, "rtems,i2c-path", &sc->path);
>      >         if (len == -1){
>      > -               device_printf(sc->dev, "Path not found in Device
>     Tree");
>      > -               OF_prop_free(sc->path);
>      > -               return (ENXIO);
>      > +               len = OF_getprop_alloc(node, "rtems,path",
>     &sc->path);
>      > +               if (len == -1) {
>      > +                       device_printf(sc->dev, "Path not found in
>     Device Tree");
>      > +                       OF_prop_free(sc->path);
>     If OF_get_prop_alloc failed, is sc->path valid here? Or do you
>     free(NULL) maybe.
> 
> OF_getprop_alloc doesn't allocate a buffer on missing properties and 0 
> length
> properties. In other cases, if it internally fails(in OF_getprop) it 
> frees the buffer and
> then returns.
> In this case, if no path property exists we just free a null buffer.
> 

So if I understand you correctly, we either get a valid pointer with len 
!= -1 or we get NULL? Sounds like the OF_prop_free(...) wasn't necessary 
before your change and isn't necessary now. So you might remove it.

Best regards

Christian

> 
>      > +                       return (ENXIO);
>      > +               }
>      >         }
>      >
>      >         if ((sc->sc_iicbus = device_add_child(sc->dev, "iicbus",
>     -1)) == NULL) {
>      > --
>      > 2.17.1
>      >
>      > _______________________________________________
>      > devel mailing list
>      > devel at rtems.org <mailto:devel at rtems.org>
>      > http://lists.rtems.org/mailman/listinfo/devel
>     <http://lists.rtems.org/mailman/listinfo/devel>
> 
> 
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>