On Wed, 2020-09-09 at 11:18 +0800, lixiaokeng wrote: > > On 2020/9/9 0:35, Martin Wilck wrote: > > > > What about this: > > > > assemble_map() is only called from setup_map(), which sets mp- > > >features > > in select_features(). So what we should do is check for NULL after > > select_features(), or have that function return an error code if > > strdup > > fails, and bail out early in setup_map() in that case. > > > > Then we simply need to add a comment in assemble_map() saying that > > mp->features must be non-null when this function is called. > > > > As I said earlier, I'm of course not against checking function > > parameters, but here we should fail to setup a "struct multipath" > > in > > the first place in setup map(), rather than returning an > > incompletely > > initialized one. If we handle it this way, we don't need to check > > the > > fields of struct multipath over and over again. Similar arguments > > hold > > for other structs. > > > > Of course this kind of assumption needs to be better documented in > > the > > code. > > > > Regards > > Martin > > > > > Hi Martin, > If I don't misunderstand, we check feature after select_features > and > return 1 if feature is NULL in setup_map. We delete strdup and add > message "mp->features must be non-null" in assemble_map. Like this: > --- > select_features(conf, mpp); > if (!mpp->featrues) > return 1; > > --- > /* mp->feature must not be NULL */ > APPEND(p, end, "%s %s %i %i", mp->features, mp->hwhandler > --- Yes. I guess error handling in setup_map() needs a closer look, too. Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
