is const necessary in eg int compar(const void *, const void *)

[lovecreatesbeauty]

Is the keyword const necessary in the comparison function in qsort and
bsearch?

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

If the pointer cannot be dereferenced why worry if the pointed object
will be modified?

 

[Ian Collins]

Is the keyword const necessary in the comparison function in qsort and
bsearch?

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

If the pointer cannot be dereferenced why worry if the pointed object
will be modified?

Why can't the pointer be dereferenced?

 

[Peter Nilsson]

Is the keyword const necessary in the comparison function
in qsort and bsearch?

Not strictly necessary, but prudent.

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

If the pointer cannot be dereferenced why worry if the
pointed object will be modified?

When the pointer(s) are converted to the appropriate object
type, they certainly can be dereferenced, and usually are.

 

[lovecreatesbeauty]

Not strictly necessary, but prudent.



When the pointer(s) are converted to the appropriate object
type, they certainly can be dereferenced, and usually are.

Thank you.

I ignored this, the const qualifier on the original pointers will
still require the same qulifier to be applied on those appropriate
object.

 

[lovecreatesbeauty]

(e-mail address removed)0m said:

The pointer *can* be dereferenced after conversion to the appropriate type.
For example, look at all the dereferencing going on here:

Yes, thanks. I got it.

  if(0 == diff)
  {
    diff =
    (left->tm_mon > right->tm_mon) - (left->tm_mon < right->tm_mon);
  }
  if(0 == diff)
  {
    diff =
    (left->tm_mday > right->tm_mday) - (left->tm_mday < right->tm_mday);
  }

why don't you put your code together and create another same selection
block

 

[Nate Eldredge]

Thank you.

I ignored this, the const qualifier on the original pointers will
still require the same qulifier to be applied on those appropriate
object.

Hmm, but wait a minute. I think your original point was valid.

For a function like

void foo(const int *p) { /* ... */ }

the compiler will complain if you attempt to assign to, or otherwise
modify, *p. So the compiler helps you make good on the promise you're
making to the caller, that *p will not be modified; this is the whole
point of const. You could circumvent this by assigning to *(int *)p,
but at least the fact that you had to introduce a cast serves as warning
that you might be doing something wrong.

But for a function like

void bar(const void *p) { /* ... */ }

where p is known to point to an int, you *have* to cast p before you can
do much with it. And if you're going to cast it, you could just as
easily cast it to `int *' as to `const int *', and the compiler will (in
my tests) remain silent either way. So the help you get from using
const is much less.

Now if we also have

void qux(void *);

and `bar' attempts to call `qux(p)', the compiler does whine. (This in
fact is exactly the whine that occurs if you pass a function taking
`void *' arguments to `qsort'.) But certainly a lot of the protection
has been lost. One could argue that this makes the use of
`const void *' sort of pointless, but if nothing else it provides some
documentation, I suppose.

 

[Nate Eldredge]

Nate Eldredge said:



Not so:

#include <stdio.h>

void bar(const void *p)
{
const int *pi = p;
printf("%d\n", *pi);
} /* look, ma, no casts */

Aha. Whereas assigning p to a variable of type `int *' does result in a
diagnostic. Got it.

So therefore it would be good practice in functions accepting `const
void *' arguments to assign to a temporary pointer of the appropriate
type, rather than casting, in order to preserve the usefulness of
const. Your function is thus to be preferred to

void bar(const void *p)
{
printf("%d\n", *(const int *)p);
}

since the latter habit, in a larger function, might let you
inadvertently omit const from the cast, and perhaps modify *p.

 

[James Kuyper]

Yes, thanks. I got it.


why don't you put your code together and create another same selection
block

If the value of diff was 0 at the first if(), then diff gets
recalculated, in which case it might be non-0 for the second if(), so
you can't combine the two blocks into a single if() block.

 

[Antoninus Twink]

I'm of the opinion that it's always possible, and usually preferable,
to avoid casting.

As usual, your blind dogmatism leads you to talk nonsense.

A couple of trivial examples that come quickly to mind:

1) When passing a NULL pointer to variadic functions, you need an
explicit cast, e.g.
s = mystrconcat("hello", " ", "world", "!", (char *) NULL);

2) If you have a type that you know is an integer type but is typedef'd
in a system-specific header, then to portably print it you need a cast
to the widest integer type:

opaque_integer_type i = somefunction(42);
printf("The answer is %llu\n", (unsigned long long) i);

 

[Richard Tobin]

1) When passing a NULL pointer to variadic functions, you need an
explicit cast, e.g.
s = mystrconcat("hello", " ", "world", "!", (char *) NULL);

I'm sure someone will point out how it *can* be done without a cast,
but of course a cast is the natural way to do it.

2) If you have a type that you know is an integer type but is typedef'd
in a system-specific header, then to portably print it you need a cast
to the widest integer type:

opaque_integer_type i = somefunction(42);
printf("The answer is %llu\n", (unsigned long long) i);

Alternatively the implementor can provide a macro for a format string,
as C99 does (PRIdMAX etc).

-- Richard

 

[Nate Eldredge]

Richard Tobin said:


Mr Twink is wrong on technical grounds and sub-optimal on stylistic
grounds: firstly, you *don't* need a cast, and secondly, "explicit" is
redundant.

You're thinking of something like

char *n = NULL;
s = mystrconcat("hello", " ", "world", "!", n);

I assume?

Yes. I don't think anyone has claimed otherwise.


Yes. But if the type is opaque and not supported by the implementation, the
cast is in any case unwise. Instead, you should pass the value to the
opaque type's stream-writing function.

It's a pleasant world you live in where such things always exist.

Unix, for instance, is rife with integer types that don't come with
output functions. pid_t, uid_t, half the fields of struct stat, etc.
So I tend to do

printf("The file has inode number %ju\n", (uintmax_t)st.st_ino);

(st.st_ino has type `ino_t').

Of course, I can avoid the cast by saying

uintmax_t inode = st.st_ino;
printf("The file has inode number %ju\n", inode);

 

[lovecreatesbeauty]

If the value of diff was 0 at the first if(), then diff gets
recalculated, in which case it might be non-0 for the second if(), so
you can't combine the two blocks into a single if() block.- Hide quoted text -

Yes, but I don't think the original code is a good piece.

#include <time.h>

int cmp_tm(const void *vleft, const void *vright)
{
const struct tm *left = vleft;
const struct tm *right = vright;
int diff =
(left->tm_year > right->tm_year) - (left->tm_year < right->tm_year);
if(0 == diff)
{
diff =
(left->tm_mon > right->tm_mon) - (left->tm_mon < right->tm_mon);
}
if(0 == diff)
{
diff =
(left->tm_mday > right->tm_mday) - (left->tm_mday < right->tm_mday);
}
/* hour, min, sec similarly */

return diff;

}

#include <time.h>

int cmp_tm(const void *v1, const void *v2)
{
const struct tm *t1 = v1;
const struct tm *t2 = v2;
int diff_y, diff_m, diff_d;

diff_y = t1->tm_year != t2->tm_year;
diff_m = t1->tm_mon != t2->tm_mon;
diff_d = t1->tm_mday != t2->tm_mday;

if (!diff_y && !diff_m){
/* usage of diff_d; */
}

/* hour, min, sec similarly */

return diff_d; /*diff in sec maybe*/
}

 

[James Kuyper]

Yes, but I don't think the original code is a good piece.

#include <time.h>

int cmp_tm(const void *vleft, const void *vright)
{
const struct tm *left = vleft;
const struct tm *right = vright;
int diff =
(left->tm_year > right->tm_year) - (left->tm_year < right->tm_year);
if(0 == diff)
{
diff =
(left->tm_mon > right->tm_mon) - (left->tm_mon < right->tm_mon);
}
if(0 == diff)
{
diff =
(left->tm_mday > right->tm_mday) - (left->tm_mday < right->tm_mday);
}
/* hour, min, sec similarly */

return diff;

}

#include <time.h>

int cmp_tm(const void *v1, const void *v2)
{
const struct tm *t1 = v1;
const struct tm *t2 = v2;
int diff_y, diff_m, diff_d;

diff_y = t1->tm_year != t2->tm_year;
diff_m = t1->tm_mon != t2->tm_mon;
diff_d = t1->tm_mday != t2->tm_mday;

if (!diff_y && !diff_m){
/* usage of diff_d; */
}

/* hour, min, sec similarly */

return diff_d; /*diff in sec maybe*/
}

A comparison function intended for use by qsort() or bsearch() must
return a result that is either positive, zero, or negative, depending
upon the relative order of the two arguments. Your function always
returns a value of 1 if the two arguments are not equal; it never
returns a negative value.

Your code doesn't seem to handle the cases where diff_y!=0, or
diff_m!=0; at least not correctly, even if we ignore the sign issue.

The if statements in Richard Heathfield's version allow it to skip
unnecessary calculations. It's not clear to me what your version would
look like if you corrected the problems I've already pointed out. It
seems, however, that the fundamental difference between his version and
your version is that your version performs all of the possible
comparisons, whether or not they're needed, and then tests them multiple
times. Even if the inefficiency of such a small function isn't important
in your application, I think Richard's approach is much cleaner.

 

[lovecreatesbeauty]

A comparison function intended for use by qsort() or bsearch() must
return a result that is either positive, zero, or negative, depending
upon the relative order of the two arguments. Your function always
returns a value of 1 if the two arguments are not equal; it never
returns a negative value.

Your code doesn't seem to handle the cases where diff_y!=0, or
diff_m!=0; at least not correctly, even if we ignore the sign issue.

Thanks for pointing out this. I was not aware that that function was
intended to be the comparision function of qsort or bsearch. (I'm also
sorry to misunderstand Richard Heathfield's code.)

The if statements in Richard Heathfield's version allow it to skip
unnecessary calculations. It's not clear to me what your version would
look like if you corrected the problems I've already pointed out. It
seems, however, that the fundamental difference between his version and
your version is that your version performs all of the possible
comparisons, whether or not they're needed, and then tests them multiple
times. Even if the inefficiency of such a small function isn't important

is it better to use this simple line:
d[0] = t1->tm_year - t2->tm_year;

instead of Richard Heathfield's:
diff = (left->tm_year > right->tm_year) - (left->tm_year < right-

tm_year);

My new version attached Thanks for read.

#include <time.h>

#define CNT 3 /* or 6, plus hour, min, sec fields */

int cmp_tm(const void *v1, const void *v2)
{
const struct tm *t1 = v1;
const struct tm *t2 = v2;
int diff, d[CNT], i;

d[0] = t1->tm_year - t2->tm_year;
d[1] = t1->tm_mon - t2->tm_mon;
d[2] = t1->tm_mday - t2->tm_mday;

/* hour, min, sec similarly */

for (i = 0; i < CNT; i++)
if (!(diff = d))
break;
return diff;
}

 

[Ian Collins]

is it better to use this simple line:
d[0] = t1->tm_year - t2->tm_year;

instead of Richard Heathfield's:
diff = (left->tm_year > right->tm_year) - (left->tm_year < right-

tm_year);

Richard's technique is intended to remove the risk of integer over
(under?) flow.

 

[James Kuyper]

Thanks for pointing out this. I was not aware that that function was
intended to be the comparision function of qsort or bsearch.

Take a look back to the first message of this discussion. The very first
sentence specifies that qsort() and bsearch() are the context for the
question that is the Subject: line in this discussion.

....

is it better to use this simple line:
d[0] = t1->tm_year - t2->tm_year;

instead of Richard Heathfield's:
diff = (left->tm_year > right->tm_year) - (left->tm_year < right-

tm_year);

For "reasonable" tm_year values, your approach is perfectly fine.
However, if you want your function to have no easily avoidable undefined
behavior, regardless of what inputs are passed to it, then you have to
consider the possibility that the mathematical value of t1->tm_year -
t2->tm_year might be greater than INT_MAX, or less than INT_MIN. If that
is the case, the subtraction has undefined behavior. In practice, on
many systems a large positive mathematical difference will cause d[0] to
have a negative value, and vice-versa.

Richard's version avoids that problem; the result of the subtraction is
always -1, 0, or 1.

My new version attached Thanks for read.

#include <time.h>

#define CNT 3 /* or 6, plus hour, min, sec fields */

int cmp_tm(const void *v1, const void *v2)
{
const struct tm *t1 = v1;
const struct tm *t2 = v2;
int diff, d[CNT], i;

d[0] = t1->tm_year - t2->tm_year;
d[1] = t1->tm_mon - t2->tm_mon;
d[2] = t1->tm_mday - t2->tm_mday;

/* hour, min, sec similarly */

for (i = 0; i < CNT; i++)
if (!(diff = d))
break;
return diff;
}

Other than the issue raised above, this looks OK to me, though I might
be missing something. I'd make diff=d part of the loop condition,
removing the need for the if() and the break, but many people would
disapprove of my approach as being hard to read and understand.

Your approach is marginally less efficient than Richard Heathfield's, in
that it performs calculations whose results will often not be needed.
However, in terms of clarity I think it's a toss-up: your loop helps
avoid RH's repeated if() statements.

 

[jaysome]

Nate Eldredge said:



I'm of the opinion that it's always possible, and usually preferable, to
avoid casting.

If p is of any type other than void *, can (and should) this cast be
avoided?

printf("p = %p\n", (void*)p);

Example:

#include <stdio.h>
int main(void)
{
double pi = 355 / 113.0; /* close enough for some purposes */
printf("pi is %f and &pi is %p\n", pi, (void*)&pi);
return 0;
}

 

[CBFalconer]

.... snip ...

If p is of any type other than void *, can (and should) this cast
be avoided?

printf("p = %p\n", (void*)p);

The variadic functions, in particular printf and clones, are
glaring examples of exceptions to the general rule "casts are
errors". The reason is that the only type information such
functions have is the construct of the format string, which is not
generally known at compile time (the format string may be a
variable).

In your example, above, the %p has specified that the functions is
receiving a void* parameter. So, if p is not of this type, you
have to convert it with a cast. A good compiler will at least tell
you if you are making an illegal cast, such as casting an integer
to a void*.

 

[Ben Bacarisse]

... A good compiler will at least tell
you if you are making an illegal cast, such as casting an integer
to a void*.

What is "illegal" about it? The conversion is implementation defined,
but illegal seems way too strong a word.

 

[vippstar]

What is "illegal" about it? The conversion is implementation defined,
but illegal seems way too strong a word.

The conversion is implementation defined, and one of the possible
behaviors is undefined behavior.
If something is UB on an imaginary implementation it's a good enough
reason for me to avoid that 'something'.
Chuck could've just been more clear by using a term defined in the
standard.