On Fri, Oct 20, 2017 at 10:55:01AM +0200, [ext] Andreas Reichel wrote: > On Thu, Oct 19, 2017 at 06:20:17PM +0200, Jan Kiszka wrote: > > On 2017-10-19 16:49, [ext] Andreas J. Reichel wrote: Can we please apply this patch now? [ This is 2/3 ]
> > > From: Andreas Reichel <[email protected]> > > > > > > If bgenv_get is called with a NULL pointer as data destination, it > > > returns the size of the buffer that must be provided by the user. > > > > > > However, if the function fails, it also returns an error code greater > > > than zero, that overlaps possible values for the buffer size, so that > > > the user cannot distinguish failed and successful states. > > > > > > To solve this, error codes are mapped to their negative counterparts, > > > i.e. bgenv_get returns -errno instead of errno. > > > > > > To be symmetric to bgenv_set, the same is changed there. > > > > > > Signed-off-by: Andreas Reichel <[email protected]> > > > --- > > > env/env_api_fat.c | 24 ++++++++++++------------ > > > env/uservars.c | 4 ++-- > > > include/ebgenv.h | 5 +++-- > > > tools/tests/test_api.c | 8 ++++---- > > > 4 files changed, 21 insertions(+), 20 deletions(-) > > > > > > diff --git a/env/env_api_fat.c b/env/env_api_fat.c > > > index 0635024..851b70c 100644 > > > --- a/env/env_api_fat.c > > > +++ b/env/env_api_fat.c > > > @@ -454,11 +454,11 @@ int bgenv_get(BGENV *env, char *key, char *type, > > > void *data, uint32_t maxlen) > > > char buffer[ENV_STRING_LENGTH]; > > > > > > if (!key || maxlen == 0) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > e = bgenv_str2enum(key); > > > if (!env) { > > > - return EPERM; > > > + return -EPERM; > > > } > > > if (e == EBGENV_UNKNOWN) { > > > if (!data) { > > > @@ -526,7 +526,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void > > > *data, uint32_t maxlen) > > > if (!data) { > > > return 0; > > > } > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > return 0; > > > } > > > @@ -539,12 +539,12 @@ int bgenv_set(BGENV *env, char *key, char *type, > > > void *data, uint32_t datalen) > > > char *value = (char *)data; > > > > > > if (!key || !data || datalen == 0) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > > > > e = bgenv_str2enum(key); > > > if (!env) { > > > - return EPERM; > > > + return -EPERM; > > > } > > > if (e == EBGENV_UNKNOWN) { > > > return bgenv_set_uservar(env->data->userdata, key, type, data, > > > @@ -555,10 +555,10 @@ int bgenv_set(BGENV *env, char *key, char *type, > > > void *data, uint32_t datalen) > > > val = strtol(value, &p, 10); > > > if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || > > > (errno != 0 && val == 0)) { > > > - return errno; > > > + return -errno; > > > } > > > if (p == value) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > env->data->revision = val; > > > break; > > > @@ -572,10 +572,10 @@ int bgenv_set(BGENV *env, char *key, char *type, > > > void *data, uint32_t datalen) > > > val = strtol(value, &p, 10); > > > if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || > > > (errno != 0 && val == 0)) { > > > - return errno; > > > + return -errno; > > > } > > > if (p == value) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > env->data->watchdog_timeout_sec = val; > > > break; > > > @@ -583,15 +583,15 @@ int bgenv_set(BGENV *env, char *key, char *type, > > > void *data, uint32_t datalen) > > > val = strtol(value, &p, 10); > > > if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || > > > (errno != 0 && val == 0)) { > > > - return errno; > > > + return -errno; > > > } > > > if (p == value) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > env->data->ustate = val; > > > break; > > > default: > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > return 0; > > > } > > > diff --git a/env/uservars.c b/env/uservars.c > > > index abbcf54..cd3f24b 100644 > > > --- a/env/uservars.c > > > +++ b/env/uservars.c > > > @@ -96,7 +96,7 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char > > > *type, void *data, > > > uservar = bgenv_find_uservar(udata, key); > > > > > > if (!uservar) { > > > - return EINVAL; > > > + return -EINVAL; > > > } > > > > > > bgenv_map_uservar(uservar, &lkey, <ype, &value, NULL, &dsize); > > > @@ -136,7 +136,7 @@ int bgenv_set_uservar(uint8_t *udata, char *key, char > > > *type, void *data, > > > p = bgenv_uservar_alloc(udata, total_size); > > > } > > > if (!p) { > > > - return errno; > > > + return -errno; > > > } > > > > > > bgenv_serialize_uservar(p, key, type, data, total_size); > > > diff --git a/include/ebgenv.h b/include/ebgenv.h > > > index 4343cef..bb5b3c5 100644 > > > --- a/include/ebgenv.h > > > +++ b/include/ebgenv.h > > > @@ -58,7 +58,8 @@ int ebg_env_get(ebgenv_t *e, char *key, char* buffer); > > > * @param e A pointer to an ebgenv_t context. > > > * @param key name of the environment variable to set > > > * @param value a string to be stored into the variable > > > - * @return 0 on success, errno on failure > > > + * @return 0 on success, -errno on failure. If buffer is NULL, > > > + * the required buffer size is returned. > > > */ > > > int ebg_env_set(ebgenv_t *e, char *key, char *value); > > > > > > @@ -68,7 +69,7 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value); > > > * @param datatype user specific string to identify the datatype of the > > > value > > > * @param value arbitrary data to be stored into the variable > > > * @param datalen length of the data to be stored into the variable > > > - * @return 0 on success, errno on failure > > > + * @return 0 on success, -errno on failure > > > */ > > > int ebg_env_set_ex(ebgenv_t *e, char *key, char *datatype, uint8_t > > > *value, > > > uint32_t datalen); > > > diff --git a/tools/tests/test_api.c b/tools/tests/test_api.c > > > index fc5267d..d6b12e2 100644 > > > --- a/tools/tests/test_api.c > > > +++ b/tools/tests/test_api.c > > > @@ -104,7 +104,7 @@ static void test_api_accesscurrent(void **state) > > > assert_int_equal(ret, 0); > > > > > > ret = ebg_env_set(&e, "kernelfile", "vmlinuz"); > > > - assert_int_equal(ret, EPERM); > > > + assert_int_equal(ret, -EPERM); > > > > > > will_return(bgenv_open_latest, &env); > > > ret = ebg_env_open_current(&e); > > > @@ -112,7 +112,7 @@ static void test_api_accesscurrent(void **state) > > > > > > assert_int_equal(ebg_env_set(&e, "kernelfile", "vmlinuz"), 0); > > > assert_int_equal(ebg_env_set(&e, "kernelparams", "root=/dev/sda"), 0); > > > - assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "abc"), > > > EINVAL); > > > + assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "abc"), > > > -EINVAL); > > > assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "0013"), 0); > > > assert_int_equal(ebg_env_set(&e, "ustate", "1"), 0); > > > > > > @@ -121,7 +121,7 @@ static void test_api_accesscurrent(void **state) > > > assert_int_equal(ret, 0); > > > > > > ret = ebg_env_get(&e, "kernelfile", buffer); > > > - assert_int_equal(ret, EPERM); > > > + assert_int_equal(ret, -EPERM); > > > > > > will_return(bgenv_open_latest, &env); > > > ret = ebg_env_open_current(&e); > > > @@ -282,7 +282,7 @@ static void test_api_uservars(void **state) > > > ret = ebg_env_set_ex(&e, "A", "B", dummymem, space_left); > > > free(dummymem); > > > > > > - assert_int_equal(ret, ENOMEM); > > > + assert_int_equal(ret, -ENOMEM); > > > > > > // test user data type > > > ret = ebg_env_set_ex(&e, "A", "B", "C", 2); > > > > > > > Is that a regression of a previous patch that is still in next, thus > > should be fold that, or an additional topic? Same for the other patch. > > > To be on the same side: "A regression of a previous patch" means to me, > that a previous patch introduced an error in an existing code, which > worked before. > > Then: No, this is not a regression. The same for the other patch. > > Andreas > > > Jan > > -- > Andreas Reichel > Dipl.-Phys. (Univ.) > Software Consultant > > [email protected], +49-174-3180074 > TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring > Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller > Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082 > > -- > You received this message because you are subscribed to the Google Groups > "EFI Boot Guard" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/efibootguard-dev/20171020085501.GA17185%40iiotirae. > For more options, visit https://groups.google.com/d/optout. -- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant [email protected], +49-174-3180074 TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082 -- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/20171102115337.GB5374%40iiotirae. For more options, visit https://groups.google.com/d/optout.
