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. > > -Petri > > > > _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
