memset_s() and abort_handler_s() missed some standard notes
ClosedPublic

Authored by yuripv_gmx.com on Aug 11 2017, 7:14 PM.

Details

Summary

abort_handler_s() currently simply calls abort(), though the documentation says it needs to do more work - "Writes an implementation-defined message to stderr which must include the string pointed to by msg and calls abort()."

memset_s() is missing the fact that it should treat "n > smax" condition as error, and invoke the constraint handler after filling the buffer - "following errors are detected at runtime and call the currently installed constraint handler function after storing ch in every location of the destination range [dest, dest+destsz) if dest and destsz are themselves valid", one of the errors is "n > smax" itself.

Test Plan

Tested using the updated test case.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
yuripv_gmx.com created this revision.Aug 11 2017, 7:14 PM
cem added a subscriber: cem.

Can you upload a diff with context (-U9999) please? Thanks!

kib added inline comments.Aug 12 2017, 9:17 AM
lib/libc/stdlib/set_constraint_handler_s.c
89 ↗(On Diff #31966)

Should abort_handler_s() be async-signal safe ? There is no direct answer to the question in the standard, but intent is clearly to allow for almost mechanical replacement of the 'unsafe' (they are not, but I have no better term) functions by a 'safe' function (again, they are not).

memset(3) is async-signal safe, so we perhaps want the memset_s() to be useful in a signal context or be safe against cancellation.

abort() has contradictory requirements already, on the one hand C11 and POSIX claim that it must be async-signal safe, on the other hand POSIX says 'may include an attempt to effect fclose( ) on all open streams' and then admits that it is somewhat silly.

Our implementation of abort does try to call FILEs flush. More, __throw_constrain_handler_s() locks a mutex. Still, I am not sure that this is a good reason to add more async-signal unsafe code.

TL;DR version: I would very much prefer this fprintf call to be replaced with two write(2) calls.

lib/libc/string/memset_s.c
45 ↗(On Diff #31966)

You do not need braces there.

Hopefully I got your comments right.

kib added inline comments.Aug 12 2017, 2:05 PM
lib/libc/stdlib/set_constraint_handler_s.c
90 ↗(On Diff #31981)

Yes, this is what I mean.

I would write this the '18' s line as

{
    static const char msg[] = "abort_hand.... : ";
...
   (void) _write(STDERR_FILENO, msg, sizeof(msg) - 1);
...

Updated, thanks for handholding!

kib accepted this revision.Aug 12 2017, 2:35 PM
This revision is now accepted and ready to land.Aug 12 2017, 2:35 PM
This revision was automatically updated to reflect the committed changes.