maptime.c warnings

[hobu]

gcc -c -g -O2 -fPIC -Wall  -DENABLE_STDERR_DEBUG -DNEED_NONBLOCKING_STDERR  
-DHAVE_VSNPRINTF -DNEED_STRLCAT  -DUSE_EPPL -DUSE_PROJ -DUSE_WMS_SVR -DUSE_WMS_LYR 
-DUSE_WFS_SVR -DUSE_WFS_LYR       -DUSE_GD_GIF -DUSE_GD_PNG -DUSE_GD_JPEG 
-DUSE_GD_WBMP -DUSE_GD_FT -DGD_HAS_FTEX_XSHOW -DGD_HAS_GDIMAGEGIFPTR 
-DGD_HAS_GETBITMAPFONTS   -DUSE_SDE  -DUSE_OGR -DUSE_GDAL  -DUSE_ICONV    
 -DUSE_ZLIB  -I/usr/include   -I/usr/local/include    -I/usr/sde/sdeexe92/include 
-I/usr/local/include           maptime.c -o maptime.o
maptime.c: In function ‘msStrptime’:
maptime.c:148: warning: implicit declaration of function ‘strptime’
maptime.c:148: warning: return makes pointer from integer without a cast

The code does this:

#if defined(_WIN32) && !defined(__CYGWIN__)
/* we need to provide our own prototype on windows. */
char *strptime( const char *buf, const char *format, struct tm *timeptr );
#endif

char *msStrptime(const char *s, const char *format, struct tm *tm)
{
#if defined(_WIN32) && !defined(__CYGWIN__)
    /* we are now using a local strptime found strptime.c */
    return strptime(s, format, tm);
#else
    /* Use system strptime() on non-windows systems */
    return strptime(s, format, tm);
#endif
}

I'm compiling on linux FC4 here, and it seems we don't have the strptime prototype here either. Also that #ifdef seems kind of silly, using the same function call for both windows and unix.

I propose it look like:

/* we need to provide our own prototype */
char *strptime( const char *buf, const char *format, struct tm *timeptr );

char *msStrptime(const char *s, const char *format, struct tm *tm)
{
    return strptime(s, format, tm);
}

full patch below

Index: maptime.c
===================================================================
--- maptime.c   (revision 6503)
+++ maptime.c   (working copy)
@@ -133,20 +133,12 @@
 }
 #endif
 
-#if defined(_WIN32) && !defined(__CYGWIN__)
-/* we need to provide our own prototype on windows. */
+/* we need to provide our own prototype */
 char *strptime( const char *buf, const char *format, struct tm *timeptr );
-#endif
 
 char *msStrptime(const char *s, const char *format, struct tm *tm)
 {
-#if defined(_WIN32) && !defined(__CYGWIN__)
-    /* we are now using a local strptime found strptime.c */
     return strptime(s, format, tm);
-#else
-    /* Use system strptime() on non-windows systems */
-    return strptime(s, format, tm);
-#endif

[sdlime]

CC'ing Assefa...

[tomkralidis]

I've applied this patch and maptime.c builds with cleanly. I also tested against msautotest/wms_time*.map with 100% success.

I'm on FC6; can't speak for other OS's though.

[tomkralidis]

(FYI the patch failed, so I manually applied the changes to maptime.c)

[dmorissette]

'man strptime' says that glibc2 needs one of _XOPEN_SOURCE or _GNU_SOURCE defined. Adding this indeed solves the strptime warning:

--- maptime.c   (revision 6654)
+++ maptime.c   (working copy)
@@ -26,6 +26,7 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/

+#define _GNU_SOURCE  /* glibc2 needs this for strptime() */
 #include <stdlib.h>
 #include <stdio.h>
 #include <time.h>

It seems to me that this would be better than hardcoding the prototype in the source.

[dmorissette]

Fixed in r6658 using _GNU_SOURCE in maptime.c as described above.

I also removed the #ifdef inside msStrptime() as suggested in the initial report.

[hobu]

I think we still have to hardcode the prototype for Windows though

[dmorissette]

Correct, and I didn't touch it.

Note that in the windows case the function comes from strptime.c and a better approach would have been to place the prototype in a header that is common to both strptime.c and maptime.c instead of hardcoding it directly in maptime.c file as was done here.