getenv_s calls error handler for "normal" errors

[bossmc]

Prior to d424528, 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 <d424528>, 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 <#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 <disclaimer.h> 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