[Patch, libfortran] PR 47802 Implementation of CTIME intrinsic

[Janne Blomqvist]

On Wed, Feb 23, 2011 at 20:23, Tobias Burnus
<tobias.burnus@physik.fu-berlin.de> wrote:
> (I think one should deal with HP-UX's _REENTRANT issue as follow up,
> cf. PR 47802, comment 17 and 18.)

As you have seen from comments 19 and 20, it's not as easy as just
defining that macro and be done with it. Suggestions welcome. Then
again, we have used localtime_r and gmtime_r since may 2009 (see PR
40190) so it might make sense to delay fixing this (whatever the fix
is) until stage 1 opens again.

> I was wondering whether there are platforms which do not have
> HAVE_STRFTIME. And if so, whether one shouldn't continue to fall
> back to "ctime" in that case.

strftime is in C89, just like ctime. So I suspect if one is available
so is the other. And, if some embedded target with an extremely
limited libc breaks a legacy instrinsic like ctime or fdate, IMHO
that's not worth worrying about.

>  fdate_sub (char * date, gfc_charlen_type date_len)
> [...]
> +  fctime (date, date_len, &now);
>
> As convenient as I find that one directly passes the buffer:
> Doesn't this reduce the number of characters effectively
> by one?

Yes. Is this worth fixing? It would be easy to fix by using malloc to
allocate a tmp array of size date_len + 1, and then call f_strcpy to
copy the result. As the fix is easy, I'm thinking it should be done,
but I have no particularly hard opinion.

> Assume 'Sat Aug 19 18:13:14 1995'. That's 24 characters.
> If one now does:
>  character(len=24) :: str
>  call fdate(str)
> will this work? I fear that this will produce
> 'Sat Aug 19 18:13:14 199\0'.

AFAICS it will produce a string filled with blanks. From POSIX 2008
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/strftime.html):

RETURN VALUE

    If the total number of resulting bytes including the terminating
null byte is not more than maxsize, these functions shall return the
number of bytes placed into the array pointed to by s, not including
the terminating NUL character. Otherwise, 0 shall be returned and the
contents of the array are unspecified.


> +   not available, gmtime use thread-local storage so it's
>
> s/use/uses/

Thanks.

> libgfortran/intrinsics/time_1.h
> +static struct tm *
> +localtime_r (const time_t * timep, struct tm * result)
> +{
> +  *result = *localtime (timep);
> +  return result;
> +}
>
> Any reason that you do not mark it as inline?

Ah, indeed, now that this fallback is moved from a .c file to a
header, it needs to be "static inline".

> Additionally, I would like to see the following documentation
> changes, partially unrelated to your patch:
> http://gcc.gnu.org/onlinedocs/gfortran/FDATE.html
> http://gcc.gnu.org/onlinedocs/gfortran/CTIME.html
>
> - CTIME/FDATE: Mention as a note hat the string might be
>  localized if setlocale had been called.
>
> - CTIME: Document that TIME also accepts INTEGER(4)
>  (Actually, it internally only accepts integer(8),
>   but integer(4) is converted by the FE.)
> - CTIME: Cross reference to DATE_AND_TIME and FDATE
> - FDATA: Cross reference to DATE_AND_TIME and CTIME

Yes, I have planned to do this once the patch itself is in.



-- 
Janne Blomqvist