Do you recommend I should rewrite:
int compare_strings(void *a,void *b) {
char *s1 = a;
char *s2 = b;
return strcmp(s1,s2);
}
Firstly, if you're looking to save keystrokes, you are going about it in
a funny way, because the following has the casts /and/ is shorter:
int compare_strings(void *a, void *b)
{
return strcmp((char *) a, (char *) b);
}
If you think the cast is too much work, why don't you think that writing
Absolutely, you should have the casts. You have an unsafe conversion here.
For excellent reasons, the above isn't valid when interpreted as C++.
The problem is, the compiler won't help you if you do want the casts,
but forget them.
I would recommend that the void * type simply be avoided in favor of something
else:
int sort_function(int (*comp)(const unsigned char *, const unsigned char *),
... args);
Now this requires a diagnostic, and you have to put in the casts:
int compare_strings(const unsigned char *a, const unsigned char *b) {
const char *s1 = a;
const char *s2 = b;
return strcmp(s1,s2);
}
The unsigned char * is the best generic pointer-to-any-object in C.
as
int compare_strings(void *a,void *b) {
char *s1 = (char *)a;
char *s2 = (char *)b;
return strcmp(s1,s2);
}
If so, why? If not, why is the void pointer from malloc any different?
The void pointer from malloc is different because you didn't compute it,
malloc did.
You can abuse the comparison function in ways that you cannot abuse the
object from malloc. The object from malloc is always suitably aligned; the only
thing that might be wrong with it is that it might have the wrong size.
(Plus the oddity that if you accidntally request zero bytes, you might get
a null pointer, or might not).
But you can take an array of structures and pass it to qsort with a
comparison function that compares them as something else, such as char *
strings. No diagnostics are required from the program, and there is no way to
restruture the program to obtain diagnostics, without using something other
than qsort.
You can't write a wrapper for sort because qsort doesn't pass through a context
pointer which could be used to chain from a generic comparison function to one
which has a safer type.
If you are using sqort, the only way that the pointer can be just converted to
char * is if you are sorting an array of char arrays, e.g.
char array[][10] = { "foo", "bar", ... };
If you want to sort an array of dynamic strings, i.e. array of char *, you
would have to have:
int compare_fun(const void *le, const void *ri)
{
char **left = (char **) le;
char **right = (char **) ri;
return strcmp(*left, *right);
}
A newbie, or maybe even a not-so-newbie, could easily make mistake here
and just convert the pointers to char *.
The casts don't prevent the mistake, but the casts flag all the places in the
program where dubious manipulation of objects may be taking place.
You can locate these places and review them.
That's a fair trade: you are allowed to do what you want. But you have to
pay for it by writing a little textual token which flags it in a way
that a simple assignment does not.
The cast is like the the ``high voltage'' label on an electric transformer,
or ``poison'' warning on a bottle.
People who argue against casts are essentially arguing that such labels should
be removed. ``Hey, /I/ know what I put in that bottle! If you don't know what
you put in, then you have a process problem that won't be solved by labelling
bottles!''
