on truncation put a 0 at the end, shorten the length by -1. len is the size then.
snprintf_sm, vsnprintf_s, snwprintf_s, vsnwprintf_s

Several _s functions were adopted into C11, Annex K.

Document them and describe differences. e.g. they use the much bigger RSIZE_MAX (half address space), we the smaller RSIZE_MAX_MEM (256MB) or RSIZE_MAX_STR (4KB).

See e.g. https://stackoverflow.com/questions/33604857/why-is-rsize-t-defined

Hello,

I tried to build the kernel module but it seems broken:

• slkm_init.c includes ../safeclib/safeclib_private.h which is not found (this include should be replaced by safeclib_private.h and Kbuild should add -I$(src)/src in ccflags-y)
• even with this update, time.h (and wchar.h) are included in most of the header files but are not available in kernel space.

Best Regards,

Fabrice

I think I found a bug. From memcpy_s description in the standard:

If there is a runtime-constraint violation, the memcpy_s function stores zeros in the first
s1max characters of the object pointed to by s1 if s1 is not a null pointer and s1max is
not greater than RSIZE_MAX.

But in the current memcpy_s, the destination buffer is not set to zeros even if both the dst and dstMax are valid. This will require splitting apart these two max comparisons.

I haven't checked other functions for the same failure.

sec_api/string_s.h, sec_api/wchar_s.h

EXTERN errno_t _strset_s(char *_Dst,  size_t _DstSize,  int _Value);
EXTERN errno_t _strlwr_s(char *_Str, size_t _Size);
EXTERN errno_t _strnset_s(char *_Str, size_t _Size, int _Val, size_t _MaxCount);
EXTERN errno_t _strupr_s(char *_Str, size_t _Size);
EXTERN errno_t _wcserror_s(wchar_t *_Buf, size_t _SizeInWords, int _ErrNum);
EXTERN errno_t _wcsnset_s(wchar_t *_Dst, size_t _DstSizeInWords, wchar_t _Val, size_t _MaxCount);
EXTERN errno_t _wcsset_s(wchar_t *_Str, size_t _SizeInWords, wchar_t _Val);
EXTERN errno_t _wcslwr_s(wchar_t *_Str, size_t _SizeInWords);
EXTERN errno_t _wcsupr_s(wchar_t *_Str, size_t _Size);

we don't do the locale specific funcs

According to standard:
If no runtime-constraint violation occurred, the vsprintf_s function returns the number of characters written in the array, not counting the terminating null character. If an encoding error occurred, vsprintf_s returns a negative value. If any other runtime-constraint violation occurred, vsprintf_s returns zero.

Scenario "vsprintf_s: len exceeds dmax" is run-time constraint violation & not encoding error so the return value should be 0.

Link to the code instance: https://github.com/rurban/safeclib/blob/master/src/str/vsprintf_s.c#L147

Hi rurban,

According to your implementation, I wonder why not to check runtime-constraints sort by priority? Such as about strcpy_s, NULL pointer(ESNULLP) issue should be the highest priority, because that will cause process crashes, ESZEROL, ESLEMAX, ESOVRLP, or ESNOSPC should have lower priority, they just cause feature is not working. What do you think?

strcpy_s (char * restrict dest, rsize_t dmax, const char * restrict src)
{
.......
if (unlikely(dest == NULL)) {
.......
return RCNEGATE(ESNULLP);
}
if (unlikely(dmax == 0)) {
........
return RCNEGATE(ESZEROL);
}
if (unlikely(dmax > RSIZE_MAX_STR)) {
.......
}
if (unlikely(src == NULL)) {
......
return RCNEGATE(ESNULLP);
}
........
}

Documentation notes that %s and %c must include both a pointer to buffer and a size of buffer as 2 parameters per token. However, the implementation appears to not handle this properly, and actually works the same as previous scanf, with just a pointer to buffer. If adding the size parameter, results in a crash.

../../../src/safeclib_private.h:191:10: fatal error: 'safe_str_constraint.h' file not found. wrong -I

and several src files are missing in the dist: make distcheck

clean the docs: add missing wchar, remove doc/html from the dist. we only need the manpages.

It is defined in the sources, but we need to add a generated safe_config.h
with configured, user-definable overrides, esp. for the RSIZE_MAX_* values.

• DISABLE_EXTS (not in C11, Annex K) (?, was --enable-exts)
• RSIZE_MAX_MEM
• RSIZE_MAX_STR
• SAFECLIB_STR_NULL_SLACK
• STRTOK_DELIM_MAX_LEN 16

See https://rurban.github.io/safeclib/doc/safec-3.0/index.html for the current status.

str/wchar/multibyte:

• vsnwprintf_s (done, but inherently unsafe)
• vswprintf_s (done)
• snwprintf_s (done, but inherently unsafe)
• swprintf_s (done)
• snprintf_s (added, but inherently unsafe)
• vsnprintf_s (added, but inherently unsafe)
• vsprintf_s (added)
• vwprintf_s (done)
• wprintf_s (done)
• sscanf_s (done)
• vsscanf_s (done)
• wctomb_s (done)
• wcstombs_s (done)
• mbstowcs_s (done)
• strerror_s (done)
• strerrorlen_s (done)
• fwscanf_s (done)
• swscanf_s (done)
• vfwscanf_s (done)
• vswscanf_s (done)
• vwscanf_s (done)
• wscanf_s (done)
• fwprintf_s (done)
• vfwprintf_s (done)
• wcscpy_s (done)
• wcsncpy_s (done)
• wcscat_s (done)
• wcsncat_s (done)
• wcstok_s (done)
• wcsnlen_s (done)
• wcrtomb_s (done)
• mbsrtowcs_s (done)
• wcsrtombs_s (done)

mem/byte:

• wmemcpy_s (done)
• wmemmove_s (done)

io: (all done)

• fopen_s
• fprintf_s
• freopen_s
• fscanf_s
• scanf_s
• vfscanf_s
• printf_s
• vfprintf_s
• vprintf_s
• gets_s

os:

• getenv_s
• asctime_s (done)
• ctime_s (done)
• gmtime_s
• localtime_s

misc:

• bsearch_s
• qsort_s

Currently the error messages are customized to include the function name as prefix, and the argument name.
This is great for the user, but most likely memory overkill.
Normal libc's have a unique error message per number, stored globally only once. Not 100x times.

Measure the memory impact.
possibly convert to unified strings
or to fprintf("%s: %s message", __FUNCTION__, "dest"), so we can share at least the base message.

There can be decomposed char sequences which need to be reordered to be fully NFD and to be comparable to the same char sequence, which is already NFD.

cast pointers to be compared to (uintptr_t) as in mem_prim_move.
Comparing pointers from two separately allocated objects is forbidden as per 6.5.8 C11 except when using (in)equality.
See intel/safestringlib#18

Done together with #40

The validation check which we are having for checking the destination size equal to zero should be updated to less than equal to zero for more robustness.
As standard claims that destination size should not be equal zero (which has implicit meaning that it shouldn't be less than zero too).

probe fails when cross-compiling

#undef the run-time invoke_safe_{str,mem}_constraint_handler function calls in safe_config.h
to avoid the large strings, as in FreeBSD.

Most of the OS/libc folks complain about it, but it is useful, similar to their malloc hooks. They are only used in the error cases.

For many/most of the functions, the C11AnnexK standard says that a negative value should be returned if there is a runtime constraint violation. However, it appears the safeclib functions are returning positive numbers if they're not being compiled for __KERNEL__. In every safeclib function the error return values are wrapped as RCNEGATE(x) and in safeclib_private.h the macro is #define RCNEGATE(x) (x) when __KERNEL__ is not defined.

So maybe you all can help me with some historical perspective. Is there an OS/platform that only accepts unsigned return values for error codes? Under what scenarios should the error values not be negative? The C11 Annex K functions are defined with int as the return type, so I would assume the negative version should always be returned.

with all or most arguments already checked at compile-time, we can split up the function into a fast unchecked worker (evtl. even using the unsafe variant without _s), and the slow _chk function, which does the preliminary arg checks of those args which are not known at compile-time. currently for clang-5+ only.
memcpy_s is 10% faster if all 4 args are known, and the _chk function can be bypassed.

evtl. using static_assert for gcc, which does not have diagnose_if.
we need both, diagnose_if from #40 to be able to test it, by changing error to warning via -DTEST_BOS and the tests-bos target.
we cannot test the static assertions via gcc.

gcc has several limitations over clang:

• there's no constexpr, only const statements checks. so we cannot check the args, we can only check inside the _s function, which needs to be inlined then. or roll our own static_assert expr, usable for comma expressions, returning the checked value.
• the object_size size is NOT known for choose_expr, the compile-time check if we can use the fast or slow function.
• const or literal integer values are not checkable via constant_p, they throw errors.

When I try to compile libsafec-20180303 (or git master) on Fedora 27 on x86_64 with gcc-7.3.1-5, the build fails with the following linker error:

  CCLD     libsafec-3.3.la
/usr/bin/ld: mem/.libs/safe_mem_constraint.o: relocation R_X86_64_PC32 against undefined symbol `__x86_return_thunk' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:1136: libsafec-3.3.la] Error 1

Reverting c86bd9e fixes the issue for me.

The generated man pages, esp. the ones with mult. APIs in one file (wcsnorm_s.c) need to be improved.

• remove the files (*_s.c.3), just one per API
• cleanup mult., like wcsnorm_s

1. either create a dynamic perl fixup script,
2. or manually sync doc updates with static man pages.

We’ve noticed there’s a difference in the C11AnnexK standard between the error return values for ‘n’ and non-n versions of the sprintf functions, but the safeclib implementations don’t make this distinction.

For sprintf:

Returns
If no runtime-constraint violation occurred, the sprintf_s function returns the number
of characters written in the array, not counting the terminating null character. If an
encoding error occurred, sprintf_s returns a negative value. If any other runtime constraint
violation occurred, sprintf_s returns zero.

For snprintf:

Returns
The snprintf_s function returns the number of characters that would have been
written had n been sufficiently large, not counting the terminating null character, or a
negative value if a runtime-constraint violation occurred. Thus, the null-terminated
output has been completely written if and only if the returned value is nonnegative and
less than n.

Note that for the ‘n’ versions, a negative value is returned for runtime constraint violations, whereas the non-n versions return zero. This difference doesn’t really make sense to us, but we found a 2005 document explaining at least the need for the non-'n' versions to return a negative value for encoding errors instead of always returning zero as in the older C standard.

We notice that all of the ‘printf’ implementations in safeclib use RCNEGATE() to return an error code, which never maps to zero, so the non-n versions aren’t following the spec because they’re not returning zero. So if everyone’s ok with it, we’ll submit a PR to clarify the runtime constraint error values returned by the sprintf family of functions per the standard. We have proposed a separate issue to ask why error values are being returned as positive numbers instead of negative numbers.

Although safe_lib_errno_.h already has #define EOK ( 0 ), I assume we'll have to overload zero and define another symbol like #define SPRINTF_CONS_VIOL ( 0 ) so at least when someone reads the code they can see the what type of error we're returning.

found by @Juno-Explorer

The current implementation of C11 Annex K functions deviate a bit from the spec and the windows sec_api, the windows sec_api is a bit more relaxed with empty slen arguments than safeclib.

Get away with the #ifdef HAVE_C11 logic. See also issue #31
Check all HAVE_C11 guards in the source and tests and make it the new default. This would make the library independent on using a C11 compiler for safeclib, and still follow the specs, but allow slen=0 shortcuts without failing.

In particular C11 sec_api allows empty slen to return immediately without dest check and cleaning.
Move that slen check upwards to mimic existing behavior.
The problem is with the footnote below A zero return value implies ... that the result in s1 is null terminated. With slen=0 alone it would not be guaranteed.

This would be the new slen check, the very first.
Following the specs, unlike windows sec_api:

    if (unlikely(slen == 0 && dest && dmax)) {
        *dest = '\0';
        return EOK;
    }

or variant 2, violating the spec footnote, following the windows sec_api.

    if (unlikely(slen == 0)) {
        if (dest && dmax)
            *dest = '\0';
        return EOK;
    }

e.g. the C11 standard says:

K.3.7.1.4 The strncpy_s function

Synopsis

#define __STDC_WANT_LIB_EXT1__ 1 
#include <string.h>
         errno_t strncpy_s(char * restrict s1,
              rsize_t s1max,
              const char * restrict s2,
              rsize_t n);

Runtime-constraints

• Neither s1 nor s2 shall be a null pointer. Neither s1max nor n shall be greater than RSIZE_MAX. s1max shall not equal zero. If n is not less than s1max, then s1max shall be greater than strnlen_s(s2, s1max). Copying shall not take place between objects that overlap.
• If there is a runtime-constraint violation, then if s1 is not a null pointer and s1max is greater than zero and not greater than RSIZE_MAX, then strncpy_s sets s1[0] to the null character.

Description

• The strncpy_s function copies not more than n successive characters (characters that follow a null character are not copied) from the array pointed to by s2 to the array pointed to by s1. If no null character was copied from s2, then s1[n] is set to a null character.

All elements following the terminating null character (if any) written by strncpy_s in the array of s1max characters pointed to by s1 take unspecified values when strncpy_s returns.

Returns
The strncpy_s function returns zero if there was no runtime-constraint violation. Otherwise, a nonzero value is returned. - A zero return value implies that all of the requested characters from the string pointed to by s2 fit within the array pointed to by s1 and that the result in s1 is null terminated.

ie. there's no ESZERO violation with n=0

Also with dmax > MAX clean dest, at least dest[0] or strlen(dest), not the whole overlarge dmax.

$ git grep 'slen == 0'
src/extstr/strcasestr_s.c:    if (unlikely(slen == 0)) {
src/extstr/strcpyfld_s.c:    if (unlikely(slen == 0)) {
src/extstr/strcpyfldin_s.c:    if (unlikely(slen == 0)) {
src/extstr/strcpyfldout_s.c:    if (unlikely(slen == 0)) {
src/extstr/strcspn_s.c:    if (unlikely(slen == 0 )) {
src/extstr/strpbrk_s.c:    if (unlikely(slen == 0 )) {
src/extstr/strspn_s.c:    if (unlikely(slen == 0 )) {
src/extstr/strstr_s.c:    if (unlikely(slen == 0)) {
src/extwchar/wcslwr_s.c:    if (unlikely(slen == 0)) {
src/extwchar/wcsstr_s.c:    if (unlikely(slen == 0)) {
src/extwchar/wcsupr_s.c:    if (unlikely(slen == 0)) {
src/str/strncat_s.c:            if (unlikely(slen == 0)) {
src/str/strncat_s.c:            if (unlikely(slen == 0)) {
src/str/strncpy_s.c:    if (unlikely(slen == 0)) {
src/str/strncpy_s.c:        if (unlikely(slen == 0)) {
src/str/strncpy_s.c:            if (unlikely(slen == 0)) {
src/str/strtok_s.c:            if (unlikely(slen == 0)) {
src/str/strtok_s.c:            if (unlikely(slen == 0)) {
src/wchar/wcsncat_s.c:            if (unlikely(slen == 0)) {
src/wchar/wcsncat_s.c:            if (unlikely(slen == 0)) {
src/wchar/wcsncpy_s.c:    if (unlikely(slen == 0)) {
src/wchar/wcsncpy_s.c:            if (unlikely(slen == 0)) {
src/wchar/wcsncpy_s.c:      if (unlikely(slen == 0)) {
src/wchar/wcstok_s.c:            if (unlikely(slen == 0)) {
src/wchar/wcstok_s.c:            if (unlikely(slen == 0)) {

$ git grep HAVE_C11 src
src/extmem/memccpy_s.c:#ifdef HAVE_C11
src/extmem/memcpy16_s.c:#ifdef HAVE_C11
src/extmem/memcpy32_s.c:#ifdef HAVE_C11
src/extmem/memmove16_s.c:#ifdef HAVE_C11
src/extmem/memmove32_s.c:#ifdef HAVE_C11
src/extmem/memset16_s.c:#ifdef HAVE_C11
src/extmem/memset32_s.c:#ifdef HAVE_C11
src/extstr/strcpyfld_s.c:#ifdef HAVE_C11
src/extstr/strcpyfldin_s.c:#ifdef HAVE_C11
src/extstr/strcpyfldout_s.c:#ifdef HAVE_C11
src/extwchar/wcslwr_s.c:#ifdef HAVE_C11
src/extwchar/wcsupr_s.c:#ifdef HAVE_C11
src/mem/memcpy_s.c:#ifdef HAVE_C11
src/mem/memmove_s.c:#ifdef HAVE_C11
src/mem/memset_s.c:#if defined HAVE_MEMSET_S && defined HAVE_C11 && defined WANT_C11
src/mem/memset_s.c:#ifdef HAVE_C11

cygwin32:

extstr/strcasestr_s.c: In function 'strcasestr_s':
extstr/strcasestr_s.c:134:13: error: array subscript has type 'char' [-Werror=char-subscripts]
             if (toupper(dest[i]) != toupper(src[i])) {
             ^

and extensions:
str,extstr,mem,extstr,io,misc,os,stdlib,wchar

Most error messages can be ignored, as they are only passed to the current constraint_handler, which is by default ignoring it. Only with --enable-debug the ignore_handler uses it.

Add a global bool has_safe_str_constraint_handler and skip processing the err_msg when it is empty.
With a handler create a new message with more info and more common strings, skipping the __FUNCTION__ prefix. This saves a lot of .cstring space, and we can print some wrong sizes.
The handler is only invoked in the error case.

Test if it's really worth.

/* has_safe_str_constraint_handler check mandatory on DEBUG */
#ifndef DEBUG
# define HAS_SAFE_STR_CONSTRAINT_HANDLER has_safe_str_constraint_handler
#else
# define HAS_SAFE_STR_CONSTRAINT_HANDLER 1
#endif

                if (HAS_SAFE_STR_CONSTRAINT_HANDLER)
                    sprintf(msg, "%s: overlapping objects", __FUNCTION__);
                handle_error(orig_dest, orig_dmax, msg, ESOVRLP);

remove the -3.4 from the .pc, to ease the integration into autotool projects. searching for safec. not safec-3.4, safec-3.3, safec-3.2 ...

See PR #56
versioning was added in the 2nd earliest commit after the sf fork, in 2012.
proper versioning helps in tighten the API to a specific version. "you only get what you asked for."
problem is that only the pkg-config .pc is versioned, (the lib anyway), but not the headers.
thus installing them in parallel will need to override the incdir and the -I path.

when adding the unversioned variant, name if libsafec.pc.

So there are four options:

1. keep the versioned safe-*.pc asis.
2. maintain two .pc's, a versioned and an unversioned. I'm leaning towards this one
3. remove the versioned .pc (as done in PR #56).
4. add versioned headers also, into a seperate safec-x.y path.

provide new unw16i*.h header files, with expanded surrogate pairs.
see the wchar2 branch.

ax_valgrind_check.m4 (from autoconf-archive) creates a GNUmakefile-only snippet.
fix it also in my autoconf-archive mirror: see https://github.com/rurban/autoconf-archive/commits/valgrind
For now you have to use gmake for clean and check-valgrind.

ifneq ($(LIBTOOL),)
valgrind_lt = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=execute
else
valgrind_lt =
endif

and more ifeq.

ifeq ($(VALGRIND_ENABLED),yes)
ifeq ($$(VALGRIND_ENABLED)-$$(ENABLE_VALGRIND_$(1)),yes-yes)
ifneq ($$(TESTS),)
else ifeq ($$(VALGRIND_ENABLED),yes)

and even a define/call/endef

The only portable solution I can think of is to use include files.
Fixed in the similar AX_CODE_COVERAGE like this: http://lists.gnu.org/archive/html/autoconf-archive-maintainers/2016-07/msg00001.html

help the branch predictor skip it.

I'm just curious why the library function prototypes don't use the restrict keyword per the C11 Annex K library. I see in the configure.ac file that you're using AC_C_RESTRICT which gnu defines as:

If the C compiler recognizes the restrict keyword, don't do anything. If it recognizes only a variant spelling (__restrict, restrict, or _Restrict), then define restrict to that. Otherwise, define restrict to be empty. Thus, programs may simply use restrict as if every C compiler supported it; for those that do not, the makefile or configuration header defines it away.

So I assume there's no harm in inserting the restrict keyword for all of the char * pointers per the standard since it would either be removed or matched each compiler's keyword. I can make this change if you want.

For strncpy_s() and strncat_s() standard doesn’t speak about what to do when slen=0!! it only says that slen shouldn’t be greater than RSIZE_MAX.

I think slen = 0 should not be considered as runtime constraint violation and instead of ESZEROL we must return EOK and without modifying the destination buffer. I think strncat_s() misses this check.

I see that all of the memory operations (memcpy_s, memmove_s, memset_s) check if smax == 0 and if so then handle it as a constraint violation. However, I can't find that requirement in the C11AnnexK standard. I agree, it's stupid for a user to request copying zero bytes but I think it's still a valid operation that should have a return value of EOK (zero) instead of ESZEROL. Please let me know if I missed something in the standard. A side benefit of removing this check would be a slight performance improvement in the normal non-zero case.

FYI @Juno-Explorer

Provide an option to strip all non-C11 Annex K functions?
Or an option to strip all 16/32 variants?

error: cannot find input file: `src/Makefile.in'

# really severe build strictness also place generated object files (.o)
# into the same directory as their source files, in order to avoid
# collisions when non-recursive make is used.
AM_INIT_AUTOMAKE([1.10 no-define foreign subdir-objects dist-bzip2 dist-xz])

Looks like there are a few tags and branches.

If I want to pull something down and build/install on Linux, is there a particular branch or tag I should be using?

Is there documentation, if not I'd be happy to update the README!

Thanks!

inclusion guards (sentinels) should not be used outside of the header in which they are defined. And I think it's alright/good/expected that a header file could be #included multiple times in a chain of headers. That's why we have inclusion guards inside every header file to prevent actual multiple inclusion of the header contents. So I don't think the library should include code like the following.

#ifndef __SAFE_LIB_H__
  #include "safe_lib.h"
#endif

It should simply be #include "safe_lib.h" Inside the header file we already have:

#ifndef __SAFE_LIB_H__
#define __SAFE_LIB_H__
... file contents
#endif /* __SAFE_LIB_H__ */

Are you receiving a warning somewhere when you write the code like this? Maybe a warning that needs to be tuned/silenced?

We were adding doxygen tags to SafeLib C file comments and came across variance in naming
of function parameters in function declaration & definition.

Please refer

• mem_primitives_lib.h
  https://github.com/rurban/safeclib/blob/master/src/safeclib/mem_primitives_lib.h
• mem_primitives_lib.c
  https://github.com/rurban/safeclib/blob/master/src/safeclib/mem_primitives_lib.c

While we're at it, would there be any issue if we resolve the differences.

With newer compilers and -D_FORTIFY_SOURCE>=2 check the dmax sizes at compile-time against __builtin_object_size (=BOS) compiler builtins, similar to the glibc and Bionic FORTIFY _chk functions, with gcc and newer clang. Just at compile-time already, not at run-time.

Add compile-time errors and warnings, and run-time warnings on --enable-warn-dmax if the 2 sizes deviate.

With clang-7 even add compile-time warnings and errors, without clang-7/diagnose_if defer to run-time.
The compile-time warning is a -Wuser-defined-warnings, and may be suppressed or increased to -Werror.

• error with "dmax exceeds dest" if dmax>BOS(dest). overflow even if dmax < RSIZE_MAX
• warn with ESLEWRNG "Wrong dmax" if dmax!=BOS(dest) with --enable-warn-dmax or clang-7.

A fatalized ESLEWRNG does not clear dest.

This might catch more wrong dmax, smax args, when the object-size is known at compile-time.
It it does no harm writing them out explicitly at the user call-site, it's even better. Yes, glibc disagrees, for the additional arg pass-thru performance hit. It's one single register/stack write.
An object may suddenly be dynamically malloced, and then glibc turns insecure, we not.

e.g.

char *dest[80];
strcpy_s(dest, 79, "1");

=> "strcpy_s: wrong dmax 79, dest has size 80" warnings to the invoke_safe_str_constraint_handler, only visible if a handler is installed.

See https://android-developers.googleblog.com/2017/04/fortify-in-android.html
esp. the new diagnose_if https://clang.llvm.org/docs/AttributeReference.html#diagnose-if

don't link to safeclib_private.h in the html and esp. man pages.
grep private doc/man/man3/* should be empty.

Hi, I'm getting a compilation error in test_vfwscanf_s.c. Here is the error:

**test_vfwscanf_s.c: In function 'test_vfwscanf_s':
test_vfwscanf_s.c:70:10: error: incompatible type for argument 3 of 'vfwscanf_s'
rc = vfwscanf_s(f, NULL, NULL);
^
In file included from test_vfwscanf_s.c:10:0:
../include/safe_str_lib.h:526:1: note: expected 'va_list' but argument is of type 'void '
vfwscanf_s(FILE restrict stream,
^

From my understanding, the 3rd argument accepts is 'va_list', and it can't be null.. thoughts?

Hi rurban,

  From configure file, '--enable-unsafe'  flag will include unsafe std C11 functions, could you tell me why these functions are unsafe? based on what? thanks!

Regards
chenlilong84

We're using doxygen for auto-generating documentation and the safeclib C file comments already use /** which is a doxygen-compatible comment type so doxygen parses the comment. But doxygen throws a lot of warnings about the use of #include within the comments. Doxygen warning:

C:/proj/jci_base_libs/Private/Libraries/C11AnnexK/safeclib/memmove_s.c:43: warning: explicit link request to 'include' could not be resolved

I haven't seen this issue before so I'm not exactly sure how it should be resolved other than completely removing that comment line so that the 'synopsis' is simply the function prototype. (at that point, why not just remove the SYNOPSIS and NAME sections because they duplicate the function definition immediately below?)

While we're at it, would there be any issue if we added doxygen style tags like @brief, @param, @return to the function comment headers? The modified doxygen header comment for memmove_s would look like:

/**
 * @brief 
 *    The memmove_s function copies smax bytes from the region pointed to by 
 *    src into the region pointed to by dest. 
 * @details
 *    This copying takes place as if the smax bytes from the region pointed 
 *    to by src are first copied into a temporary array of smax bytes that does 
 *    not overlap the region pointed to by dest or src, and then the smax 
 *    bytes from the temporary array are copied into the object region to by 
 *    dest.
 *
 * @remark SPECIFIED IN
 *    ISO/IEC TR 24731, Programming languages, environments
 *    and system software interfaces, Extensions to the C Library,
 *    Part I: Bounds-checking interfaces
 *
 * @param dest[out] pointer to the memory that will be replaced by src.
 * @param dmax   maximum length of the resulting dest, in bytes
 * @param src    pointer to the memory that will be copied to dest
 * @param smax   maximum number bytes of src that can be copied
 *
 * @pre   Neither dest nor src shall be a null pointer.
 * @pre   Neither dmax nor smax shall be 0.
 * @pre   dmax shall not be greater than RSIZE_MAX_MEM.
 * @pre   smax shall not be greater than dmax.
 *
 * @return
 *    EOK        successful operation
 *    ESNULLP    NULL pointer
 *    ESZEROL    zero length
 *    ESLEMAX    length exceeds max limit
 *    If there is a runtime-constraint violation, the memmove_s function
 *      stores zeros in the first dmax characters of the region pointed to
 *      by dest if dest is not a null pointer and dmax is not greater
 *      than RSIZE_MAX_MEM.
 *
 * @relates
 *    memmove16_s(), memmove32_s(), memcpy_s(), memcpy16_s() memcpy32_s()
 *
 */
errno_t
memmove_s (void *dest, rsize_t dmax, const void *src, rsize_t smax)
{
   ...
}

so the doxygen html documentation would now look like:

whereas originally the generated documentation without tags looked like:

• detect mingw and mingw cross (for testing with wine).
• skip conflicting declarations with their non-C11 MINGW_HAS_SECURE_API:
  strtok_s, vsnprintf_s, wcstok_s
• note their extensions, esp. their _l locale versions.
• add dllimport/dllexport decl and cleanup the header mess in the sources.

./configure --enable-unsafe --host=x86_64-w64-mingw32
wchar/vswprintf_s.c:160: warning: passing arg 2 of `vswprintf' makes pointer from integer without a cast
wchar/vswprintf_s.c:160: warning: passing arg 3 of `vswprintf' from incompatible pointer type
wchar/vswprintf_s.c:160: error: too many arguments to function `vswprintf'

for the new unsafe variants, like snprintf_s, vsnprintf_s, tmpnam_s, ...

I'm concerned that there are inclusion loops. Specifically, safe_mem_lib.h and safe_str_lib.h both include safe_lib.h which then includes both of them. I assume this is because safe_lib.h defines rsize_t and the abort/ignore handlers. I would suggest that rsize_t be moved into safe_types.h and maybe the handlers need to be put in their own .c/.h files. That way, safe_mem_lib.h and safe_str_lib.h can include safe_types.h and the safe_handlers.h but that's it. Then safe_lib.h will include safe_str_lib.h and safe_mem_lib.h.

Any issue with that restructuring to eliminate inclusion loops?

The tests actually do not return FAIL on any fails.
They only checked if crashing or not, but didn't promote any functional tests error, like wrong result or wrong error code.

In particular throw ESLEMAX when smax exceeds max, before the smax>dmax check (ESNOSPC).

Broken tests or functions:

• sprintf_s (wrong tests)
• snprintf_s (wrong tests)
• memcpy_s (wrong ESNOSPC with smax)
• memcpy16_s (wrong ESNOSPC with smax, wrong test)
• memcpy32_s (wrong ESNOSPC with smax, wrong test)
• t_memcmp_s (wrong ESNOSPC with smax)
• t_memcmp16_s (wrong ESNOSPC with smax)
• t_memcmp32_s (wrong ESNOSPC with smax)
• t_memmove_s (wrong ESNOSPC with smax, wrong test)
• t_memmove16_s (wrong ESNOSPC with smax, wrong test)
• t_memmove32_s (wrong ESNOSPC with smax, wrong test)
• t_memset_s (loose tests with C11)
• t_strcpyfldout_s (wrong test)
• t_strljustify_s (fix impl - original was ok, fix 2 test)

First of all, thank you for supporting the great library.

I'm trying to use safeclib in C++11 code. My test case is very simple

#include <libsafec/safe_str_lib.h>

int main()
{
}

With GCC 5.4.0 there are lots of similar errors due to usage of restrict keyword

safe_compile.h:38:45: error: conflicting declaration ‘void* restrict’
                                       void *restrict       /* ptr */,
                                             ^
safe_compile.h:37:51: note: previous declaration as ‘const char* restrict’
 typedef void (*constraint_handler_t) (const char *restrict /* msg */,
                                                   ^

Adding #define restrict __restrict before inclusion of the header does the trick.
The solution, however, is far from being elegant. Probably this can be handled somehow during configure.

With Clang 4.0.1 there is an additional issue with errors like this

safe_str_lib.h:106:19: error: no matching literal operator for call to 'operator""_XSTR' with arguments of types 'const char *' and 'unsigned long', and no matching literal operator template
    BOS_CHK(dest) BOS_NULL(src);
                  ^
safe_compile.h:178:56: note: expanded from macro 'BOS_NULL'
    __attribute__((diagnose_if(_BOS_NULL(buf), "empty "_XSTR(buf), bos_chk_err)))

Apparently there should be space between string literal and _XSTR() macro. Otherwise C++11 compiler will treat a whole expression as user-defined literal.
This one can be solved by a simple search/replace but still it's worth mentioning I think.