2

We have some code in spatialite that looks like:

static int cmp_pt_coords (const void *p1, const void *p2)
{
    ....
}

static gaiaGeomCollPtr auxPolygNodes (gaiaGeomCollPtr geom)
{
    ....
/* sorting points by coords */
    qsort (sorted, count, sizeof (gaiaPointPtr), cmp_pt_coords);
    ....
}

This is obviously simplified - the real code can be seen at https://www.gaia-gis.it/fossil/libspatialite/artifact/fe1d6e12c2f98dff23f9df9372afc23f745b50df

The error that I'm getting from gcc (gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3)) is

/bin/bash ../../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I../.. -g  -Wall -Werror -fprofile-arcs -ftest-coverage -g -I../../src/headers   -fvisibility=hidden -g -Wall -Werror -fprofile-arcs -ftest-coverage -g -MT libsplite_la-spatialite.lo -MD -MP -MF .deps/libsplite_la-spatialite.Tpo -c -o libsplite_la-spatialite.lo `test -f 'spatialite.c' || echo './'`spatialite.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I../.. -g -Wall -Werror -fprofile-arcs -ftest-coverage -g -I../../src/headers -fvisibility=hidden -g -Wall -Werror -fprofile-arcs -ftest-coverage -g -MT libsplite_la-spatialite.lo -MD -MP -MF .deps/libsplite_la-spatialite.Tpo -c spatialite.c  -fPIC -DPIC -o .libs/libsplite_la-spatialite.o
spatialite.c: In function 'auxPolygNodes':
spatialite.c:17843:5: error: passing argument 4 of 'qsort' from incompatible pointer type [-Werror]
/usr/include/stdlib.h:761:13: note: expected '__compar_fn_t' but argument is of type 'int (*)(void *, void *)'
cc1: all warnings being treated as errors

I've looked at some previous postings:

However they don't really seem the same (or at least, the way I read the suggestions in those postings is what I think we're already doing here).

I can cast around this, using:

    qsort (sorted, count, sizeof (gaiaPointPtr), (__compar_fn_t)cmp_pt_coords);

However I don't see why that should be necessary, and I'm worried about portability to other systems. It seems like the compiler is omitting the const-s from the arguments.

Community
  • 1
  • 1
BradHards
  • 650
  • 9
  • 27
  • You are right, there is something fishy going on. Your comparison function clearly has the correct prototype. I would suspect that some of your commandline parameters make gcc implicitly change the prototypes of `static` functions. `-fvisibility=hidden` could add some attribute, for example. – Jens Gustedt Aug 15 '12 at 10:10

3 Answers3

4

That cast is perfectly fine. GCC isn't smart enough to know that __compar_fn_t is

int (*)(const void *, const void *)

so it issues a warning.

However, __compar_fn_t is not portable -- so if you don't want to use it for casting, you should probably make GCC not warn about this using an appropriate compiler flag.

Or you can see whether __compar_fn_t is defined, and if not, define it yourself:

#ifndef __GNUC__
typedef int (*__compar_fn_t)(const void *, const void *);
#endif
  • OK, although it seems ugly. The real concern is whether will work anywhere. Do I need to use some preprocessor macro magic for it to work on other compilers? – BradHards Aug 15 '12 at 09:56
  • but `__compar_fn_t` should have *const* qualified parameters, so gcc would be wrong – Jens Gustedt Aug 15 '12 at 10:06
  • @BradHards yes, corrected. It's glibc-specific AFAIK. Why can't you just not use -Werr? –  Aug 15 '12 at 10:09
  • I'd like to use -Werror to make sure I don't miss the compiler telling me about any (real) errors as all the output runs up the screen. – BradHards Aug 15 '12 at 10:16
  • @BradHards I see - then see my updated answer, there surely is a compiler flag to turn off function pointer type mismatches. –  Aug 15 '12 at 10:17
  • @BradHards or check if compare_fn_t is available. –  Aug 15 '12 at 10:18
  • I didn't know that `typedef`ing something would enter it into the preprocessor namespace as a symbol. Or did you intend to something else? – HonkyTonk Aug 15 '12 at 11:22
  • @HonkyTonk sorry, that's wrong - I intended to do something else :) see edit. –  Aug 15 '12 at 11:25
  • I couldn't find a gcc option to turn off the function pointer warning. That was a bit surprising (given the number of gcc options), but even without -Wall the warning still occurs. I guess it'll be the pre-processor option. – BradHards Aug 15 '12 at 11:31
  • Ah, makes much more sense now. I did a quick Google run after posting my comment just to make sure I hadn't missed something. :) – HonkyTonk Aug 15 '12 at 11:36
  • Thanks for this, I will probably prefix the last parameter with `(int(*)(const void*,const void*))` so that it would look like this `qsort (sorted, count, sizeof (gaiaPointPtr), (int(*)(const void*,const void*))cmp_pt_coords);` – SSH This Jan 29 '14 at 16:59
1

The error probably comes from the visibility flag that you pass to the compiler. Your are saying that all functions in that compilation unit should be hidden. For gcc this changes the function API so your comparison function is then incompatible with the one expected by qsort.

You might want to deal with 

#pragma visibility

or

__attribute__((__visibility(default)))

or similar for your comparison function.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
  • This was an interesting idea, but leaving off the visibility flag didn't change the error. I tried the __attribute__ approach as well, also no change. – BradHards Aug 15 '12 at 10:54
  • 1
    So I would dig further into the gcc options, I just tried a bit, and with only standard compilation options these things pass fine with my gcc, here. – Jens Gustedt Aug 15 '12 at 11:43
1

The reason for the warning/error is that the GCC prototype of __compar_fn_t is:

typedef int (*__compar_fn_t)(__const void *, __const void );
and not:
typedef int (__compar_fn_t)(const void *, const void *);

Therefore, in order to solve the problem, simply define your function as:
static int cmp_pt_coords (__const void *p1, __const void *p2)

user3751454
  • 41
  • 4