On Tue, 2024-11-19 at 13:20 +0100, Martin Wilck wrote: > On Mon, 2024-11-18 at 16:14 -0500, Benjamin Marzinski wrote: > > On Mon, Nov 18, 2024 at 12:18:20PM +0100, Martin Wilck wrote: > > > On Fri, 2024-11-15 at 18:22 -0500, Benjamin Marzinski wrote: > > > > > > > > I'm not completely happy with the MAPINFO_ID_IF_FOUND flag. An > > > > alternative would be to run a second libmp_mapinfo() call > > > > without > > > > MAPINFO_MPATH_ONLY to grab the name, if the first failed with > > > > DMP_EMPTY. > > > > If people think that's a better way to solve this, I can rework > > > > those > > > > patches. > > > > > > We could simply choose to always fill in this information if the > > > the caller has requested it, without an additional input flag. > > > It's > > > not > > > an expensive operation. Is there a reason not to do this? > > > > Your comments in the code said that libmp_mapinfo() will not touch > > any > > of the output parameters if it doesn't succeed. I didn't audit the > > code, > > but I can certainly imagine a situation where you passed in > > pointers > > to > > some varaibles that already had values and you didn't want those > > values > > overwritten unless libmp_mapinfo() returned DMP_OK. > > > > But I can go look and see if any callers would get messed up if > > name > > or > > uuid got set, even when the found device didn't match. > > I am pretty sure that that shouldn't happen. The memory must be > allocated anyway, and callers can't ignore the return value. But > double-checking is a good idea, of course, and we should adapt the > documentation. I just looked through the code. Except for the unit tests, I only found one call where this matters: dm_find_map_by_wwid() fills in the name if non-NULL. This is only used by cli_add_map(), where it doesn't cause an issue. Martin > > Martin >
