On Fri, Mar 29, 2019 at 09:15:46AM +0000, Christian.Gromm@xxxxxxxxxxxxx wrote:
> > > +static int set_config_and_add_link(struct mdev_link *mdev_link)
> > > +{
> > > + int i;
> > > + int ret;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > > + ret = set_config_val[i](mdev_link);
> > > + if (ret == -ENODATA) {
> > I've read the commit description but I don't really understand the
> > error codes. Here only -ENODATA is an error. But later -ENODEV
> > is success. Why not "if (ret) {" here?
> >
>
> The most_set_cfg_* functions return success, ENODEV or ENODATA.
> We want to stop only in case there is something wrong with the
> provided data. A missing device is not an issue here. In this
> case we want to keep the configuration as is and complete stuff
> once a device is being attached.
>
Yeah... But future programmers will just add whatever error codes
occur to them at the time. Now we have two error codes which are
special instead of one (twice as much chance for them to mess up).
Just do it like:
if (ret < 0 && ret != -ENODEV) {
That's a more standard way to handle this.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
