> -----Original Message----- > From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org] > Sent: Wednesday, February 04, 2015 11:50 AM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCHv4 01/18] api: odp_cpumask.h: > odp_cpumask_to_str() return chars written or error > > On 4 February 2015 at 10:27, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolai...@nsn.com> wrote: > > > > > >> -----Original Message----- > >> From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org] > >> Sent: Wednesday, February 04, 2015 11:20 AM > >> To: Savolainen, Petri (NSN - FI/Espoo) > >> Cc: lng-odp@lists.linaro.org > >> Subject: Re: [lng-odp] [PATCHv4 01/18] api: odp_cpumask.h: > >> odp_cpumask_to_str() return chars written or error > >> > >> On 4 February 2015 at 09:38, Savolainen, Petri (NSN - FI/Espoo) > >> <petri.savolai...@nsn.com> wrote: > >> > > >> > > >> >> -----Original Message----- > >> >> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp- > >> >> boun...@lists.linaro.org] On Behalf Of ext Savolainen, Petri (NSN - > >> >> FI/Espoo) > >> >> Sent: Wednesday, February 04, 2015 10:31 AM > >> >> To: ext Ola Liljedahl > >> >> Cc: lng-odp@lists.linaro.org > >> >> Subject: Re: [lng-odp] [PATCHv4 01/18] api: odp_cpumask.h: > >> >> odp_cpumask_to_str() return chars written or error > >> >> > >> >> > >> diff --git a/include/odp/api/cpumask.h > b/include/odp/api/cpumask.h > >> >> > >> index 7482899..65969c3 100644 > >> >> > >> --- a/include/odp/api/cpumask.h > >> >> > >> +++ b/include/odp/api/cpumask.h > >> >> > >> @@ -18,12 +18,20 @@ > >> >> > >> extern "C" { > >> >> > >> #endif > >> >> > >> > >> >> > >> +#include <sys/types.h> > >> >> > >> +#include <odp/config.h> > >> >> > >> + > >> >> > >> /** @addtogroup odp_scheduler > >> >> > >> * CPU mask operations. > >> >> > >> * @{ > >> >> > >> */ > >> >> > >> > >> >> > >> - /** > >> >> > >> +/** > >> >> > >> + * @def ODP_CPUMASK_BUFSIZE > >> >> > >> + * Minimum size of output buffer for odp_cpumask_to_str() > >> >> > >> + */ > >> >> > > > >> >> > > ODP_CPUMASK_STRLEN is better. Better to use term string when > it's a > >> >> > character string. > >> But is it not a character string. > >> > >> The user specifies a buffer and a buffer size. The function writes a > >> string into that buffer (if it fits into the buffer). > >> The buffer then *contains* a string (which will be shorter that the > >> buffer size value specified, e.g. because trailing CPU bits may not be > >> set and also the string length does not include the terminating null > >> char). But a string and a buffer are not the same thing. > >> > >> >> > >> >> This one is missing from v5. > >> >> > >> >> Better name it STRLEN when it used for _to_str() and _from_str() > >> function > >> >> calls. > >> I think this is a poor choice of name. The length of a string is the > >> number of characters in the string (excluding the terminating null > >> char). But we need to pass the buffer size. > >> > >> >> > >> >> char mask_str[ODP_CPUMASK_STRLEN]; > >> >> memset(mask_str, 0, sizeof(mask_str)); > >> Why is the memset() here? It is not needed. > >> > >> >> odp_cpumask_to_str(mask, mask_str, sizeof(mask_str)); > >> >> odp_cpumask_from_str(mask, mask_str); > >> Yes this is how it is done, before my patch and after my patch. > >> > >> >> > >> >> > >> >> -Petri > >> > > >> > > >> > Or ODP_CPUMASK_STRLEN_MAX or ODP_CPUMASK_STR_MAX_CHARS ... > >> > > >> > The point is that, it's defining the max number of chars (not bytes) > >> needed for the string output - and it cannot be mixed with the > >> odp_cpumask_t (buffer) size. > >> odp_cpumask_t has a separate well-defined type (no char bufs or void > >> pointers used when you are passing an odp_cpumask_t value), I see no > >> risk for mixing it with ODP_CPUMASK_BUFSIZE. > > > > > > If you reserve ODP_CPUMASK_BUFSIZE for the string API. What if we add > odp_cpumask_to_u64(), odp_cpumask_to_u8(),odp_cpumask_to_buf(), etc API > calls later on? It's better to reserve BUF for those buffers and use STR > for strings (no matter if sting initialized or not before the call, output > is still a string). > Purely hypothetical. One can invent any number of counter arguments, > that does not make them realistic or meaningful. > What if we add other odp_cpumask string calls that need some (other) > type of buffer to hold their strings? > > But I do want to make the name as easy as possible to understand and > try to see your criticism in a constructive light. > Perhaps ODP_CPUMASK_STR_BUFSIZE? Buffer size for cpumask strings. > > The buffer size is still different from the string length just as a > buffer is not a string. >
And "hypothetical" seems to be favorite counter-counter argument... We already had _to_u64() but removed it from the API. It may have a comeback (in some form), if we need to start packing these masks into messages and send those between devices (e.g. transmit portable cpumask definitions between control and data plane devices). ODP_CPUMASK_STR_SIZE would be shorter. ODP_CPUMASK_STR_XXX, with or without BUF - is understandable to me. -Petri _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
