On 24 November 2014 at 20:08, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > On 11/24/2014 09:05 PM, Bill Fischofer wrote: >> >> Returning a pointer to an internal structure should be discouraged. You >> have no idea what the caller will do with what you return so better that >> they pass in something that is filled from the internal struct. That way >> the call is independent of the implementation or any changes to it. >> > > My point was that this name already stored somewhere. You don't need to As Bill pointed out, this is really unsafe behavior. You don't know for how long the application will sit on that pointer and possibly reference it after the pktio instance has been freed (including all memory).
Actually there is a similar case when we pass pointers (e.g. strings often used for names) when creating objects. Can the implementation save that pointer (because the caller guarantees it will be valid until the object is freed) or should the implementation make a copy of such strings? strdup() is a simple way of creating that reliable copy but I don't think we are allowed to use strdup (and thus implicitly malloc) in ODP implementations (at least not linux-generic). This is really an architectural question. Do we favor simplicity or robustness? -- Ola > allocate memory for it twice. And of course if you call odp_pktio_close() > then > you can not reference to any attributes of that packet io. That call also > independent and each implementation can allocate memory for that call > differently. > > Maxim. > > >> On Mon, Nov 24, 2014 at 9:01 AM, Mike Holmes <mike.hol...@linaro.org >> <mailto:mike.hol...@linaro.org>> wrote: >> >> >> >> On 24 November 2014 06:54, Maxim Uvarov <maxim.uva...@linaro.org >> <mailto:maxim.uva...@linaro.org>> wrote: >> >> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org >> <mailto:maxim.uva...@linaro.org>> >> >> --- >> platform/linux-generic/include/api/odp_packet_io.h | 10 >> ++++++++++ >> platform/linux-generic/odp_packet_io.c | 11 +++++++++++ >> 2 files changed, 21 insertions(+) >> >> diff --git >> a/platform/linux-generic/include/api/odp_packet_io.h >> b/platform/linux-generic/include/api/odp_packet_io.h >> index 667395c..b3b7ea6 100644 >> --- a/platform/linux-generic/include/api/odp_packet_io.h >> +++ b/platform/linux-generic/include/api/odp_packet_io.h >> @@ -148,6 +148,16 @@ int odp_pktio_set_mtu(odp_pktio_t id, int >> mtu); >> */ >> int odp_pktio_mtu(odp_pktio_t id); >> >> +/* >> + * Return the interface name from a pktio handle. This is the >> + * name that was passed to the odp_pktio_open() call. >> + * >> + * @param[in] id ODP Packet IO handle. >> + * >> + * @return pointer to the iface name or NULL. >> >> >> I think it adds value to describe what may cause the NULL, also >> presumably this will always return a terminated string can an >> application rely on that? >> >> @retval Pointer to the interface name. >> @retval NULL if packet handle is invalid >> >> + */ >> +const char *odp_pktio_get_name(odp_pktio_t id); >> + >> /** >> * @} >> */ >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index c523350..9ceb989 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -542,3 +542,14 @@ int odp_pktio_mtu(odp_pktio_t id) >> >> return ifr.ifr_mtu; >> } >> + >> +const char *odp_pktio_get_name(odp_pktio_t id) >> +{ >> + pktio_entry_t *entry; >> + >> + entry = get_entry(id); >> + if (entry == NULL) >> + return NULL; >> + >> >> >> Ensuring it is a terminated string with a removable runtime assert >> might be valuable. >> Is there a max size that this should be check against in an assert? >> >> + return entry->s.name <http://s.name>; >> +} >> -- >> 1.8.5.1.163.gd7aced9 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
