On 2 January 2015 at 22:26, Mike Holmes <[email protected]> wrote: > Provide feed back on string length error, matching the handling for the other > APIs in the file. > Improve the readability of the code. > > Signed-off-by: Mike Holmes <[email protected]> > --- > platform/linux-generic/odp_coremask.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/odp_coremask.c > b/platform/linux-generic/odp_coremask.c > index 54cd333..5b4e961 100644 > --- a/platform/linux-generic/odp_coremask.c > +++ b/platform/linux-generic/odp_coremask.c > @@ -10,19 +10,21 @@ > #include <stdlib.h> > #include <string.h> > > -#define MAX_CORE_NUM 64 > +#define MAX_CORE_NUM 64 > +#define MASK_STR_LEN 18 > +#define BASE_16 16 > > > void odp_coremask_from_str(const char *str, odp_coremask_t *mask) > { > uint64_t mask_u64; > > - if (strlen(str) > 18) { > - /* more than 64 bits */ > + if (strlen(str) > MASK_STR_LEN) { > + ODP_ERR("string longer than reqired for bit mask supported"); I don't think these checks should be done (neither flavor). Maybe the user is passing in a longer string (e.g. part of the command line). For many (valid) core masks, the hex string might be shorter and still contain unrecognized trailing characters.
> return; > } > > - mask_u64 = strtoull(str, NULL, 16); > + mask_u64 = strtoull(str, NULL, BASE_16); Better to check here that the hex number is terminated by some white space character. If not report an error. If think this type of functions which parse text (which may come from the command line or a config file etc) should have a return value which indicates the success of the operation. Invalid data that comes from external sources should not be able to crash the program. Invalid data which comes from internal sources should crash the program. > > odp_coremask_from_u64(&mask_u64, 1, mask); > } > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
