On Wed, Apr 14, 2021 at 7:28 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> On Wed, Apr 14, 2021 at 01:34:23PM +0100, Ian Abbott wrote:
> > > drivers/staging/comedi/drivers/ni_routes.c:61 ni_find_route_values() warn: 'device_family' sometimes too small '8,11' size = 30
> > > 59 for (i = 0; ni_all_route_values[i]; ++i) {
> > > 60 if (memcmp(ni_all_route_values[i]->family, device_family,
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > 61 strnlen(device_family, 30)) == 0) {
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > This whole memcmp() is very strange. Why not just use:
> > >
> > > if (strncmp(ni_all_route_values[i]->family, device_family, 30) == 0)
> >
> > I think even a simple strcmp() would do as well because all the device
> > family strings and board name strings are null terminated. I don't know why
> > the magic number 30 is used here!
> >
> > The above applies similarly to ni_find_valid_routes() too.
> >
>
> I was thinking maybe ni_all_route_values[i]->family has an additional
> string on the end. For example, it could end in "_bar" and we want
> ->family "foo_bar" to match with device_family "foo"?
No that was not my intention. Looking through the code again, it
looks like my intention was to simply provide some type of a
reasonable limit for the strings that are passed into the function
that come from hard-coded family names that are in ni_660x.c and
ni_mio_common.c where they call ni_assign_device_routes. Indeed, the
string lengths are (so far) only actually 8 ("ni_660x\0") and 11
("ni_mseries\0" and "ni_eseries\0") characters. In other words, I
think my intention was to simply create a somewhat generic interface
that other ni device drivers in comedi could use. There was not a
good reason for using a magic number of 30 except that it was bigger
than either 8 or 11 and was probably large enough to always be
expected to be bigger than any other ni device family name without
being too large.
Spencer
