Page MenuHomeFreeBSD

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

Authored by yuripv_gmx.com on Aug 11 2017, 7:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 17, 2:23 AM
Unknown Object (File)
Fri, Dec 13, 6:30 PM
Unknown Object (File)
Sun, Dec 8, 7:14 AM
Unknown Object (File)
Thu, Dec 5, 4:58 PM
Unknown Object (File)
Mon, Nov 25, 1:37 PM
Unknown Object (File)
Mon, Nov 25, 6:29 AM
Unknown Object (File)
Sun, Nov 24, 6:08 PM
Unknown Object (File)
Sun, Nov 24, 12:51 PM
Subscribers

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 - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

cem added a subscriber: cem.

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

lib/libc/stdlib/set_constraint_handler_s.c
91

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

You do not need braces there.

Hopefully I got your comments right.

lib/libc/stdlib/set_constraint_handler_s.c
91

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!

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.