I'm personally not a fan of the *_s() functions. That said, we've already added some others to the base system, so there's probably no turning back.

In D12785#267382, @cy wrote:

The gets_s() has been separated out into a different file.

Would it be better to svn copy & update or a new file from scratch?

It depends. If svn diff still returns something sensible after copying, then doing a svn copy is the right approach. If the contents of the file are almost entirely different and svn diff would return more differences than what's retained, a new file is the right approach.

I'll switch to svn from git and see what the diff gives me.

Other than that, what do people think?

Looks a lot better. Thanks!

Could I persuade you to also add some basic unit testing coverage for this function?

Unfortunately FUNLOCKFILE_CANCELSAFE and FLOCKFILE_CANCELSAFE implement an ugly lock. The macros must be coded as matching pairs and to remove the goto and replace it wtih another FUNLOCFILE_CANCELSAFE would result in nasty compile errors. They are used as pairs.

I looked at replacing the macros a couple of weeks ago but I think we need to limit the scope of this revision and consider replacing these macros, used throughout stdio, in another revision. Do you agree?

#define FLOCKFILE_CANCELSAFE(fp) \

{                                                               \
        struct _pthread_cleanup_info __cleanup_info__;          \
        if (__isthreaded) {                                     \
                _FLOCKFILE(fp);                                 \
                ___pthread_cleanup_push_imp(                    \
                    __stdio_cancel_cleanup, (fp),               \
                    &__cleanup_info__);                         \
        } else {                                                \
                ___pthread_cleanup_push_imp(                    \
                    __stdio_cancel_cleanup, NULL,               \
                    &__cleanup_info__);                         \
        }                                                       \
        {

#define FUNLOCKFILE_CANCELSAFE() \

                (void)0;                                        \
        }                                                       \
        ___pthread_cleanup_pop_imp(1);                          \
}

Ok, just leave the goto as-is then. I don't think it is worth it to try to replace those macros. pthread_cleanup_push/pop are themselves implemented as paired macros (POSIX permits this), so any wrappers of those macros will be as well.

I had a chance to think about the macros on the way to $JOB just after commenting here. We can address this by using a static inline function. It's not ideal but IMO better than goto's.

We can address the macro mess properly in the rest of stdio in another revision.

This new revision includes a test. I cobbled it up last night. It hasn't been tested yet. Better to give you a chance to review it now while I test over the weekend.

In D12785#265442, @imp wrote:

You need to fix the namespace pollution, and really, make this two separate functions. The current structure tries to be too clever, and history has shown that this will result in subtle bugs and possibly a security advisory.

Addressed in a prior revision.

Are there any more comments or requested revisions?

I suppose that since there are no more comments, I'll commit this sometime after April 4 (after I get back).

Indeed it should and the update date should be updated as well. I'll get on it.