Opened 6 years ago

Last modified 6 years ago

#82 new task

Header cleanup

Reported by: goatbar Owned by: warmerdam
Priority: normal Milestone:
Component: libgeotiff Version: 1.3.0
Keywords: headers Cc:

Description

It might be worth some additional discussion of headers. I'm working on a patch to do some cleanup / include what you use (IWYU).

Can we consider string.h a requirement and remove the #ifdef HAVE_STRING_H check? I personally consider compilers/environments without required standard headers not worth supporting at this point.

e.g. cpl_serv.h has this:

#include <stdio.h>

#include <math.h>

#ifdef HAVE_STRING_H
#  include <string.h>
#endif
#if defined(HAVE_STRINGS_H) && !defined(_WIN32)
#  include <strings.h>
#endif
#ifdef HAVE_STDLIB_H
#  include <stdlib.h>
#endif

Which I think could be done as this. Keeping the two checks around strings.h is probably critical because strings.h is a non-standard BSDism.

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#if defined(HAVE_STRINGS_H) && !defined(_WIN32)
#  include <strings.h>
#endif

But looking at man strcasecmp on a mac, I see:

Their prototypes existed previously in <string.h> before they were
moved to <strings.h> for IEEE Std 1003.1-2001 (``POSIX.1'') compliance.

So maybe even just this for the cpl_serv.h case:

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#if !defined(_WIN32)
#  include <strings.h>
#endif

Or maybe better yet to only care about if strings.h is found rather than the platform.

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#if defined(HAVE_STRINGS_H)
#  include <strings.h>
#endif

Change History (2)

comment:1 by goatbar, 6 years ago

Version: 1.3.0

See also the issue in #81: #include <geo_config.h> vs #include "geo_config.h"

And r2736 says:

  • cpl_serv.h: do not include <strings.h> on _WIN32 even if HAVE_STRINGS_H is defined
  • cpl_config.h.vc: remove #define HAVE_STRINGS_H 1
  • cpl_serv.c: remove includes of string.h and strings.h. Already done by cpl_serv.h

The line about cpl_serv.c worries me. This proper header guards, it is not expensive to do IWYU and have those includes in the c file.

And, http://include-what-you-use.org/

comment:2 by goatbar, 6 years ago

From EvenR:

Windows has a strings.h file, but that is not the Unix file at all,
so that caused issues

For reference: http://stackoverflow.com/questions/4291149/difference-between-string-h-and-strings-h

I can't find anything that describes what MS Windows has for strings.h. e.g. there isn't a list functions by header section to the "C Run-Time Library Reference" here:

https://msdn.microsoft.com/en-us/library/59ey50w6.aspx

Note: See TracTickets for help on using tickets.