Re: [Patch, libfortran] PR 47802 Implementation of CTIME intrinsic

[Tobias Burnus <burnus at net-b dot de>]

Dear Janne,

I just wanted to reply to your earlier email. Now, I reply to both :-)

Janne Blomqvist wrote:
> >   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.

(Is now fixed, but I still wanted to state why I think it makes sense.)

I think one should handle the case character(len=24) as that's the most 
obvious character length (count yourself!) and is the length which other 
Fortran implementations use - for the function ctime. Supporting ctime 
as subroutine seems to be the exception (g77 supports it). The length of 
24 is explicitly mentioned (for the function) by Sun/Oracle [1] and 
implicitly in the example by Intel [2]. (gfortran uses a length of 30 in 
its example [3].)

[1] http://download.oracle.com/docs/cd/E19205-01/819-5259/aetij/index.html
[2] 
http://software.intel.com/sites/products/documentation/hpc/compilerpro/en-us/fortran/mac/compiler_f/lref_for/source_files/rfctime.htm
[3] http://gcc.gnu.org/onlinedocs/gfortran/CTIME.html


By the way, in the documentation patch: Can one remove the ", (not 
recommended)." part after the function version? It is not obvious to me 
why the subroutine is better. g77 did not have this wording and as 
written other compilers have only the function version, which makes the 
function "better".

> > 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.

One should write in the documentation that upon error (e.g. string too 
short) the result will be a blank string (subroutine) or a zero-sized 
string (function).

Janne Blomqvist wrote:
> Patch with the above improvements attached. Ok for trunk?

(I reply to the wrong patch, but I reviewed the one without memory leak ;-)


OK. Hopefully there are no surprises on rarer targets.

Thanks for the patch!

Tobias

> 2011-02-24  Janne Blomqvist<jb@gcc.gnu.org>
>
> 	PR libfortran/47802
> 	* config.h.in: Regenerated.
> 	* configure: Regenerated.
> 	* configure.ac: Remove checks for ctime and ctime_r, add check for
> 	strftime.
> 	* intrinsics/date_and_time.c (localtime_r): Move fallback
> 	implementation to time_1.h.
> 	* intrinsics/time_1.h (localtime_r): Fallback implementation.
> 	* intrinsics/ctime.c: Include time_1.h.
> 	(ctime_r): Remove fallback implementation.
> 	(strctime): New function.
> 	(fdate): Use strctime instead of ctime_r.
> 	(fdate_sub): Likewise.
> 	(ctime): Likewise.
> 	(ctime_sub): Likewise