Re: Importing inttypes methods

[Gedare Bloom <gedare at rtems dot org>]

On Fri, Jul 28, 2017 at 9:08 AM, Corinna Vinschen <vinschen@redhat.com> wrote:
> On Jul 28 07:40, Gedare Bloom wrote:
>> On Fri, Jul 28, 2017 at 7:02 AM, Aditya Upadhyay <aadit0402@gmail.com> wrote:
>> > Just give a little time. I am creating a new patch set that fix the
>> > BSD_VISIBLE problem.
>> >
>> > Thanks,
>> > Aditya
>> >
>> >
>> > On Fri, Jul 28, 2017 at 4:13 PM, Corinna Vinschen <vinschen@redhat.com> wrote:
>> >> On Jul 27 19:30, Aditya Upadhyay wrote:
>> >>> Hi,
>> >>>
>> >>> I apologize for the inconvenience caused. Actually i am new to git and
>> >>> still learning commands. I am trying my best and want to assure you
>> >>> that i will be familiar with the git soon. I will try not to repeat
>> >>> the same mistakes again in future.
>> >>> I am attaching the fresh new patches for last 4 methods with all
>> >>> possible changes. I am requesting you to please review the patches.
>> >>
>> >> Patches are fine and what we talked about, but I realized belatedly
>> >> that we have a problem with the inttypes.h header file:
>> >>
>> >> Your patch includes xlocale.h only if __POSIX_VISIBLE >= 200809.
>> >> However, you declare the _l functions unconditionally.  This will
>> >> break builds which don't define _POSIX_SOURCE to the right value.
>> >>
>> >> Additionally, these functions are BSD-only at the moment.  They are
>> >> neither in glibc nor in POSIX.
>> >>
>> >> So, what we should do here is this:
>> >>
>> >> * Include xlocale.h only if __BSD_VISIBLE.
>> >> * Declare the _l functions only if __BSD_VISIBLE, too.
>> >>
>> >> It's your choice now:  Do you want to recreate the below patches
>> >> accordingly, or shall I aplly the patches as they are, and you create
>> >> a followup patch just fixing inttypes.h?
>> >>
>> Corinna, good catch. I mentioned this issue to Joel but it dropped out
>> the bottom some how. Is it only (for example) the strtoimax_l() that
>> needs to be guarded, or also the _strtoimax_l? (I suspect only the
>> strtoimax_l, but want to be clear before the next round of patches
>> lands here.)
>
> The reentrant prototypes use locale_t, so they depend on including
> xlocale.h, too.  It's a bit uncommon but the simplest solution.
>
Since the non-reentrant version (e.g., strtoimax) wraps the re-entrant
one, then there is no support unless locale is available. Would it be
better to have a non-reentrant, nonolocale implementation in tandem
with the reentrant one, or do we not worry about it and don't support
these functions at all unless the POSIX_SOURCE is set properly and
BSD_VISIBLE?

I don't think these guards are being done in other parts of the stdlib
code where this kind of pattern exists. I looked for some similar
examples that might be useful as a template but did not easily find
anything. Any suggestions are welcome?

I would guess there is value to have a non-reentrant implementation in
the basic case, and it should be a reasonably simple matter to
refactor the existing functions to pull out a common helper for the
re-entrant and non-reentrant, since apparently the only difference is
the use of errno in the two cases.

Aditya, this would look something like....

static inline intmax_t strtoimax_helper(const char* __restrict nptr,
char** __restrict endptr, int base) {
... the first "blob" of _strtoimax_l goes here
}

intmax_t _strtoimax_l(...) {
    x = strtoimax_helper(...);
    ... reent->_errno handling code here.
}

intmax_t strtoimax(...) {
#ifdef BSD_VISiBLE
    _strtoimax_l(...)
#else
    x = strtoimax_helper(...);
    ... errno handling here.
#endif
}

Corinna, would that seem correct and desirable for this situation? Or
do I miss something.

Gedare

>
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat