2
\$\begingroup\$

I have a comparator function I use with qsort to sort an array of char* (i.e., char**), which compiles with current versions of Clang and GCC without giving errors or warnings:

static int
comparator(const void* a, const void* b)
{
#ifdef __cplusplus
    auto a_recast = const_cast<void *>(a);
    auto b_recast = const_cast<void *>(b);
    auto a_rerecast = reinterpret_cast<char **>(a_recast);
    auto b_rerecast = reinterpret_cast<char **>(b_recast);
    auto a_rererecast = const_cast<const char **>(a_rerecast);
    auto b_rererecast = const_cast<const char **>(b_rerecast);
    return std::strcmp(*a_rererecast, *b_rererecast);
#else
    return strcmp(*(const char**) a, *(const char**) b);
#endif
}

It compiles conditionally, that is, depending on whether a C or C++ compiler is used, one or the other block of code is compiled.

I offer both blocks, because this is part of a program that can be compiled alone in C, or in a larger suite of tools compiled with a C++ toolkit.

My question is about the work I had to do to in the C++ block, where I strip const, recast to char **, and reapply const.

Is there a simpler or cleaner way to do this C++-style casting, which generates the correct end results and does not result in warnings about C-style casting?

Clang, in particular, is chatty with warnings about old C-style casts, and I like my build logs to be clean, where fixes are realistic.

I also like the code to be clear and easy to and maintain. I'd also like to inline this code, which would probably be easier for the compiler if the comparator function is smaller.

If I'm doing the correct work, that's fine, but if there are better ways, I'd be interested to learn them. Thanks for your advice.

\$\endgroup\$
  • 2
    \$\begingroup\$ What's wrong with C-style-casts? And why would you use conditional compilation instead of including <string.h>? As an aside, you would use std::sort() in C++ because it's far more efficient and type-safe. \$\endgroup\$ – Deduplicator Apr 19 '18 at 2:09
  • \$\begingroup\$ This is great criticism! Please give me all you've got, I want to learn. \$\endgroup\$ – Alex Reynolds Apr 19 '18 at 4:28
  • \$\begingroup\$ Is there a good reason you need to write bilingual (C and C++) code? Usually, #ifdef __cplusplus is something you'd use in a header so that you can link C code with C++ code. I'd recommend you write this as C, and declare it with C linkage (i.e. extern "C", when __cplusplus is defined). \$\endgroup\$ – Toby Speight Apr 19 '18 at 7:35
3
\$\begingroup\$

How about this?

int comparator(const void* a, const void* b)
{
#ifdef __cplusplus
    auto a_recast = static_cast<const char* const *>(a);
    auto b_recast = static_cast<const char* const *>(b);
    return std::strcmp(*a_recast, *b_recast);
#else
    return strcmp(*(const char* const *) a, *(const char* const *) b);
#endif
}

To elaborate, the reason that you had to go through the convoluted series of dangerous casts is that you actually were stripping constness. const void* means a promise not to change whatever the void turns out to be. That is, the thing pointed at by the rightmost star is const. const char ** on the other hand means a promise not to change the individual characters, but doesn't guarantee that you won't change the direct pointers to those characters. However, it's the pointers to those characters that are pointed at by the rightmost star. Intuitively perhaps, think of char * as being in a unit by itself a c_string, and your a and b are arrays of constant c_strings.

The reason that clang warns about c style casts is precisely that they let you apply such a crazy list of operations and changing constnesses without necessarily realizing what is happening.

As an aside, as far as the compiler is concerned, it is actually only necessary to enforce constness on the pointer. That is, you could also compile with static_cast<char * const *>. I put the additional const in because when your conceptual c_string is constant, you conceptually don't want to be changing any of the characters. (And you probably want the compiler to alert you if anything tries to do that.)

\$\endgroup\$
3
\$\begingroup\$

If you need to use this in C and C++ programs, there's a mechanism to save you writing two versions. Write the C code in the normal way, but with external linkage:

#include <string.h>

int char_string_comparator(const void* a, const void* b)
{
    const char *const *const sa = a;
    const char *const *const sb = b;
    return strcmp(*sa, *sb);
}

(Note that I've avoided casts, by assigning to compatible temporary variables - any optimizing compiler should produce the same code as for your version, but there's more checking performed in this one).

Compile it to an object file, and write a header so that you can link it with either C or C++ programs:

#ifdef __cplusplus
extern "C" {
#endif

int char_string_comparator(const void* a, const void* b);

#ifdef __cplusplus
}
#endif

All that said, I have never needed std::qsort() when writing C++. Instead, use std::sort() (from <algorithm>) when something needs sorting. Because it's a template, it's correctly type-safe, and you can't use a comparator that doesn't match the data type.

\$\endgroup\$
  • \$\begingroup\$ const char *const *const sa = a; holy cow! It looks like inception :-o \$\endgroup\$ – t3chb0t Apr 19 '18 at 7:51
  • \$\begingroup\$ Okay @t3chb0t, the very last const in that type is pretty unnecessary, given we can see that we never assign to sa after it's initialized. But my fingers were on a roll by then... I suppose if I wanted to make it musical, then I could go for char const*const*const! \$\endgroup\$ – Toby Speight Apr 19 '18 at 7:58
  • 1
    \$\begingroup\$ I have no idea what this is for but it's pretty scarry. Can't you just write char const^3 - just kidding ;-) \$\endgroup\$ – t3chb0t Apr 19 '18 at 8:02
  • \$\begingroup\$ No @t3chb0t. That XORs const and 3! ;-) \$\endgroup\$ – Josiah Apr 19 '18 at 8:06
  • 1
    \$\begingroup\$ char const³, obviously. \$\endgroup\$ – Toby Speight Apr 19 '18 at 8:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.