On Mon, 2007-10-15 at 17:06 +0200, Sasha Khapyorsky wrote: > On 07:38 Mon 15 Oct , Hal Rosenstock wrote: > > Hi Sasha, > > > > On Mon, 2007-10-15 at 15:54 +0200, Sasha Khapyorsky wrote: > > > Hi Hal, > > > > > > On 06:18 Mon 15 Oct , Hal Rosenstock wrote: > > > > > > > > I don't recall as this is from a very very long time ago but in looking > > > > at this, I agree with your assessment that it can be simplified (and > > > > there appears to be no real need for what is contained in struct Port > > > > other than the fd). The only downside I see is the subtle change in the > > > > public umad_ APIs changing int portid -> int fd. > > > > > > There is no API change at all - umad_open_port() still return unique > > > integer descriptor as it was before. Here we are only changing > > > undocumented (at least I'm not able to find any public description about > > > what umad_open_port() should return) behavior of this API (by replacing > > > mad device number as umad_open_port() return value, > > > > It's all the other APIs which say umad_xxx(int portid, ...) are now > > umad_xxxx(int fd, ...). A subtle change. > > I changed this only in umad.c files (to make it clear for internal > implementation reviewers) and saved it as 'portid' in the header where > API is described - an user should not care what internal meaning of > portid is. For getting fd explicitly there is umad_get_fd(portid) > method.
Which could be eliminated as redundant; not sure anything is using this API. > > > but if we want to > > > support multiple open()s there is no choice - device number is not > > > suitable for this). > > > > Understood. > > > > > > I suppose all the tools > > > > would continue to work without change here even if libibumad were > > > > changed underneath it, right ? > > > > > > Right. > > > > > > > BTW, when you do this, the umad man pages > > > > should all be updated for this change. > > > > > > I see only that umad_open_port.3 should be fixed - it says that return > > > value is "0" on success, which is not correct anyway. Not really related > > > to the patch. Do you see another places to fix in man? > > > > Don't a number of them indicate int portid as an input parameter (and > > this should now be int fd) ? Just grep for portid in those man pages... > > Don't think we want to make the internal in its nature "portid = fd > feature" to be part of the public API. 'portid' is fine IMO because it > doesn't mean a lot - just "0 or an unique positive value...", pretty > suitable for public API. portid gives a level of abstraction but is it needed ? If we were starting today, would you say the same thing ? -- Hal > Sasha > _______________________________________________ > general mailing list > [email protected] > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general > > To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
