[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[McIDAS #LRH-227193]: Incorrect typing of 3rd parameter to Mcdaytimetosec



Hi DaveP, DaveS, and Rick,

DaveP said:
> We actually did exactly what Tom is suggesting; change time_t to int.  I don't
> think we ever tried to be exhaustive in the search/replace, we just fixed the
> ones that jumped out at the time or that failed testing.

That is exactly what I did to begin with... I ran into problems serving GVAR
data off of unidata2.ssec.wisc.edu when I built McX v2007x in 64-bit mode.  I
reported weird behavior to the Data Center folks when I first saw the problem
thinking that it was related to the data being sent via LDM to unidata2.  When
DC folks, including Dee, sent back a note that indicated that some things worked
and some things didn't, I figured that the problem was with the newly rebuilt
code.  I rebuilt in 32-bit mode and the problems went away.  That's when I dug
into the gvaradir.cp and gvaraget.cp code.  Voila, I found the 'time_t *' typing
of parameters to Mcdaytimetosec while the function declaration in daytime.c
specified an 'int *'.

I didn't look any farther until I discussed the situation with Scott L. and
Russ D. on Tuesday morning last week.  Scott did a quick grep of source code
and noticed the problem in poesadir.cp and poesaget.cp.  When I returned from
Madison, I did made a complete survey of the source code and found the errors
in just under half of the routines that called Mcdaytimetosec.  While looking
through daytime.c, I noticed that the comment section for Mcdaytimetosec 
incorrectly
stated the need for use of a 'time_t *' as the third argument.  I then verified
that the man page for Mcdaytimetosec called for a 'time_t *' for the third
argument.  I also must note that the Remarks section of the Mcdaytimetosec
documentation is incorrect:

*$      This function should work fine until approximately the year
*$      2099. At that time the number of seconds since 1 Jan 1970 will
*$      no longer fit in a 32 bit unsigned integer.

This would be true if the parameter used was an 'unsigned int'.  Since it
is _not_ an 'unsigned int', the 'stop working' year is 2038.

> I don't know if "time_t" is technically *incorrect*, it's just that some
> compilers/headers don't have correct size definitions for time_t (and size_t) 
> on
> 64bit platforms.  I think.

The use of a 'time_t *' variable is only incorrect because the function 
declaration
(and function prototype in mcidas.h) specifies use of an 'int *'.  If there was
no Fortran interface, then use of 'time_t *' would have been preferable.

Since the size of a 'time_t *' is implementation dependent, one can not assume 
that
it will be the same size as an 'int *' which is pretty standard.  It is 
possible that
use of a 'time_t *' is "OK" (meaning it works) on some 64-bit systems and 
incorrect
(meaning it doesn't work) on others. Not passing the type called for in the 
function
declaration is an error no matter what.

DaveS asked:
> DaveP: Didn't we have problems with Fortran calling C on 64-bit when the
> C routine expected time_t? How was is resolved?

It appears that someone rewrote the Mcdaytimetosec routine so that the Fortran
interface to it, mcdaytimetosec_, could pass along the argument that it 
received.
It seems to me that whoever rewrote Mcdaytimetosec (or first implemented it) did
it the hard way: it would have been easier to populate a 'tm' structure with
the information passed in and then call 'mktime'.  Since the return value from
'mktime' is a 'time_t', one would have to convert back to an 'int' so that the
Fortran interface would work correctly.

By the way, I implemented all of the changes included in the list that I sent
to DaveS and rebuilt Unidata McIDAS-X on all of the systems that I have access
to.  I have seen no problems with the modifications on any of the systems 
involved
and those include:

32-bit machines
----------------
cacimbo.ggy.uga.edu            - RedHat Enterprise WS 4 Linux
buran.ggy.uga.edu              - MacOS-X
weather.admin.niu.edu          - Fedora 7 Linux
weather2.admin.niu.edu         - Fedora 7 Linux
weather3.admin.niu.edu         - Fedora 7 Linux
zeldagillroy.silversprings.net - Fedora Core 6 Linux (my home development 
machine)

64-bit machines
-------------------
yakov.unidata.ucar.edu         - Fedora Core 5 Linux
adde.ucar.edu (aka motherlode) - SunOS 5.9 SPARC
unidata2.ssec.wisc.edu         - SunOS 5.8 SPARC
stratus.al.noaa.gov            - SunOS 10 x86_64
idd.unl.edu                    - Fedora Core 6 Linux

Since adde.ucar.edu, unidata2.ssec.wisc.edu, and stratus.al.noaa.gov get a LOT
of exercise serving data via ADDE, I am pretty confident that the fixes
(time_t -> int) have had no unintended side effects.

Cheers,

Tom
****************************************************************************
Unidata User Support                                    UCAR Unidata Program
(303) 497-8642                                                 P.O. Box 3000
address@hidden                                   Boulder, CO 80307
----------------------------------------------------------------------------
Unidata HomePage                       http://www.unidata.ucar.edu
****************************************************************************


Ticket Details
===================
Ticket ID: LRH-227193
Department: Support McIDAS
Priority: Normal
Status: Closed


NOTE: All email exchanges with Unidata User Support are recorded in the Unidata inquiry tracking system and then made publicly available through the web. If you do not want to have your interactions made available in this way, you must let us know in each email you send to us.