`getenv_s` calls error handler for "normal" errors

[bossmc]

Prior to d42452838ad2054fde06c493eab1a1787abc6fc5, getenv_s would only call the error handler callback if the failure was caused by one of the safety checks (buffers too long/short, NULL pointers etc) but in that change it was changed to call into handle_error in the "Variable not found" error case (seemingly in order to invoke the "Null Slack" function to clear dbuf).

It seems to me that calling the callback only makes sense for the safety checks (and this seems to be consistent with behaviour elsewhere including in the other changes in that commit (e.g. asctime_s).

I've not done a complete inspection to see if there are other cases where "normal errors" are triggering the handler, since it's possible that this is deliberate (in which case, I guess this issue raises the opposite issue 😄 ).

[jleffler]

The C11 standard says

§K.3.6.2.1 The getenv_s function ( http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1):

      #define __STDC_WANT_LIB_EXT1__ 1

      #include <stdlib.h> <http://port70.net/~nsz/c/c11/n1570.html#7.22>

      errno_t getenv_s(size_t * restrict len,

                 char * restrict value, rsize_t maxsize,

                 const char * restrict name);

Runtime-constraints

2 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p2 name shall not be a null pointer. maxsize shall neither equal zero nor be greater than RSIZE_MAX. If maxsize is not equal to zero, then value shall not be a null pointer.

3 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p3 If there is a runtime-constraint violation, the integer pointed to by len is set to 0 (if len is not null), and the environment list is not searched.

Description

4 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p4 The getenv_s function searches an environment list, provided by the host environment, for a string that matches the string pointed to by name.

5 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p5 If that name is found then getenv_s performs the following actions. If len is not a null pointer, the length of the string associated with the matched list member is stored in the integer pointed to by len. If the length of the associated string is less than maxsize, then the associated string is copied to the array pointed to by value.

6 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p6 If that name is not found then getenv_s performs the following actions. If len is not a null pointer, zero is stored in the integer pointed to by len. If maxsize is greater than zero, then value[0] is set to the null character.

7 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p7 The set of environment names and the method for altering the environment list are implementation-defined. The getenv_s function need not avoid data races with other threads of execution that modify the environment list.408) http://port70.net/~nsz/c/c11/n1570.html#note408

Returns

8 http://port70.net/~nsz/c/c11/n1570.html#K.3.6.2.1p8 The getenv_s function returns zero if the specified name is found and the associated string was successfully stored in value. Otherwise, a nonzero value is returned.

Footnotes

408) http://port70.net/~nsz/c/c11/n1570.html#note408 Many implementations provide non-standard functions that modify the environment list.

The function should not call the runtime constraint violation handler unless one of the runtime constraints is violated.

A non-existent environment variable is not a runtime constrain violation.

On Mon, Jul 11, 2022 at 11:33 AM Andy Caldwell @.***> wrote:

Prior to d424528 https://github.com/rurban/safeclib/commit/d42452838ad2054fde06c493eab1a1787abc6fc5, getenv_s would only call the error handler callback if the failure was caused by one of the safety checks (buffers too long/short, NULL pointers etc) but in that change it was changed to call into handle_error in the "Variable not found" error case (seemingly in order to invoke the "Null Slack" function to clear dbuf.

It seems to me that calling the callback only makes sense for the safety checks (and this seems to be consistent with behaviour elsewhere including in the other changes in that commit (e.g. asctime_s)).

I've not done a complete inspection to see if there are other cases where "normal errors" are triggering the handler, since it's possible that this is deliberate (in which case, I guess this issue raises the opposite issue 😄 ).

— Reply to this email directly, view it on GitHub https://github.com/rurban/safeclib/issues/119, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCAHBWYIUV434U5EJHBWSTVTRLINANCNFSM53IIRD2A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Jonathan Leffler @.***> #include Guardian of DBD::Informix - v2018.1031 - http://dbi.perl.org "Blessed are we who can laugh at ourselves, for we shall never cease to be amused."

[bossmc]

Thanks for the confirmation @jleffler!

I've raised #120 for the change needed to fix this issue.

[rurban]

Thanks! Merged