On 28 January 2015 at 13:28, Savolainen, Petri (NSN - FI/Espoo) <[email protected]> wrote: > > >> -----Original Message----- >> From: ext Ola Liljedahl [mailto:[email protected]] >> Sent: Wednesday, January 28, 2015 2:03 PM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: LNG ODP Mailman List >> Subject: Re: [lng-odp] odp_cpumask.h >> >> On 28 January 2015 at 12:55, Savolainen, Petri (NSN - FI/Espoo) >> <[email protected]> wrote: >> > >> > >> >> -----Original Message----- >> >> From: ext Ola Liljedahl [mailto:[email protected]] >> >> Sent: Wednesday, January 28, 2015 1:29 PM >> >> To: Savolainen, Petri (NSN - FI/Espoo) >> >> Cc: LNG ODP Mailman List >> >> Subject: Re: [lng-odp] odp_cpumask.h >> >> >> >> On 28 January 2015 at 12:21, Savolainen, Petri (NSN - FI/Espoo) >> >> <[email protected]> wrote: >> >> > >> >> > >> >> >> -----Original Message----- >> >> >> From: [email protected] [mailto:lng-odp- >> >> >> [email protected]] On Behalf Of ext Ola Liljedahl >> >> >> Sent: Wednesday, January 28, 2015 12:48 PM >> >> >> To: LNG ODP Mailman List >> >> >> Subject: [lng-odp] odp_cpumask.h >> >> >> >> >> >> * @param str Output buffer >> >> >> * @param len Size of string length (incl. ending zero) >> >> >> */ >> >> >> void odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int >> len); >> >> >> >> >> >> Why don't we use size_t for the"len" parameter? >> >> >> >> >> >> -- Ola >> >> > >> >> > >> >> > It's an object size, so it can be changed to size_t (similar to e.g. >> >> snprintf()). >> >> > >> >> > Packet data len/offsets, buffer sizes, packet/bytes counts etc, >> should >> >> be uintxx_t - but object sizes should be size_t. >> >> > >> >> > >> >> > Answer to: What happens if len < output length? Maybe we copy the >> >> snprintf definition (except output also ending zero). Return number >> chars >> >> written (incl. str ending zero), otherwise return number of chars that >> >> would have been outputted (return >len and clip output). >> >> This is more complicated to check for and goes against the convention >> >> of returning <0 for errors. >> >> I would prefer my suggestion with returning -(minimum required buffer >> >> size), there's a clear separation between success and error. >> >> >> >> And that we add a define with a suggested buffer size which will be >> >> usable when the CPU mask is full (all bits set), this seems to be 128 >> >> when using Linux cpu_set_t for the implementation. Of course ODP could >> >> use a smaller max size as well. >> > >> > >> > About error from functions that return len. I'd have input param and >> return type the same, so that compiler don't complain when comparing those >> (maybe your audio was breaking on call yesterday on that part). >> No I didn't hear, my wifi router stopped working and had to be reset... >> >> > >> > e.g. >> > >> > size_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, size_t >> len); >> Better to use ssize_t then? > > The point is that the same type is used for "len" and return value. > Explicitly signed type for len would not be intuitive. > >> >> > >> > >> > size_t len, out; >> > char str[SIZE]; >> > >> > len = sizeof(str); >> > >> > out = odp_cpumask_to_str(mask, str, len); >> > >> > if (out > len) { >> Simple yes. But we just defined the convention that we use negative >> values to indicate failures. Why not follow that here? >> To use positive values to indicate both success and error conditions >> seems confusing to me. snprintf is not a good example here. > > The same, unsigned type - so that compiler does not complain when comparing > those two. > > size_t a; > ssize_t b; > > if (a > b) > > gcc -W -Wall ... > > main.c:17:8: warning: comparison between signed and unsigned integer > expressions [-Wsign-compare] OK seems this warning is not included in -Wall but is included in -W. I need to take this into consideration.
> if (a > b) > > > -Petri > > _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
