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.

>
>
> -Petri
>
>
>
>

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to