comparison between signed and unsigned

[evanevankan2]

I have a question about the warning 'comparison between signed and
unsigned' I get from my code. It comes from the conditional in the
outer for loop.
I understand the warning, but I'm not sure what is the best way to
prevent it. I can just change i to a type size_t or maybe put a cast
in the conditional, but I don't know which way that is 'best'?
Any ideas? I provided the code below for some context.

And while we're at it, could you please check if the code looks good?

And last question, the signed integer in the comparison will promoted
to unsigned, right?
Could you give some examples on what could go wrong when comparing
signed and unsigned integers?

Thanks

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static void *fail_malloc(size_t size)
{
void *ptr = malloc(size);

if (ptr == NULL) {
fprintf(stderr, "Memory allocation error\n");
exit(EXIT_FAILURE);
}

return ptr;
}

typedef int insertion_compare(const void *insertion_a, const void
*insertion_b);

void insertion_sort(void *data, size_t nmemb, size_t size,
insertion_compare *cmp)
{
int i, j;
char *ptr = data;
char *tmp = fail_malloc(size);

for (i = 1; i < nmemb; i++) {
memcpy(tmp, ptr + size * i, size);

for (j = i - 1; j >= 0 && cmp(ptr + size * j, tmp) > 0; j--)
{
memmove(ptr + size * (j + 1), ptr + size * j, size);
}

memcpy(ptr + size * (j + 1), tmp, size);
}
free(tmp);
}

 

[vippstar]

I have a question about the warning 'comparison between signed and
unsigned' I get from my code. It comes from the conditional in the
outer for loop.
I understand the warning, but I'm not sure what is the best way to
prevent it. I can just change i to a type size_t or maybe put a cast
in the conditional, but I don't know which way that is 'best'?
Any ideas? I provided the code below for some context.

And while we're at it, could you please check if the code looks good?

And last question, the signed integer in the comparison will promoted
to unsigned, right?

Only if all the values of the unsigned integer cannot be correctly
represented by the signed integer type.

Could you give some examples on what could go wrong when comparing
signed and unsigned integers?

for (j = i - 1; j >= 0 && cmp(ptr + size * j, tmp) > 0; j--)

<snip>
cmp() is not defined anywhere. That's a constraint violation and your
code won't compile. Post compilable code.

 

[vippstar]

I'm sorry for my previous reply in which I said 'cmp' is not defined
anywhere. It's defined in the functions definition as a pointer to a
function.

I have a question about the warning 'comparison between signed and
unsigned' I get from my code. It comes from the conditional in the
outer for loop.
I understand the warning, but I'm not sure what is the best way to
prevent it. I can just change i to a type size_t or maybe put a cast
in the conditional, but I don't know which way that is 'best'?
Any ideas? I provided the code below for some context.

Do you want to change your code? It's just a warning, comparing signed
with unsigned doesn't invoke undefined behavior in any way.

And while we're at it, could you please check if the code looks good?

And last question, the signed integer in the comparison will promoted
to unsigned, right?

As I said before, only if the unsigned integers value cannot be
correctly represented by the signed integer type.

Could you give some examples on what could go wrong when comparing
signed and unsigned integers?

Nothing. Though the results, while defined, might be not the ones you
expect, for instance:
10u > -1
is not true. -1 gets promoted to unsigned int, and becomes UINT_MAX.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static void *fail_malloc(size_t size)
{
void *ptr = malloc(size);

if (ptr == NULL) {
fprintf(stderr, "Memory allocation error\n");
exit(EXIT_FAILURE);

What if previous allocations succeded? Memory leak.

}

return ptr;

}

typedef int insertion_compare(const void *insertion_a, const void
*insertion_b);

void insertion_sort(void *data, size_t nmemb, size_t size,
insertion_compare *cmp)
{
int i, j;

If you change these to size_t, your second for loop won't work,
specifically the part j >= 0, which is always true, since j would be
size_t, an unsigned integer.
<snip>

 

[evanevankan2]

On Jul 13, 5:17 pm, (e-mail address removed) wrote:> I have a question about the warning 'comparison between signed and



Only if all the values of the unsigned integer cannot be correctly
represented by the signed integer type.

Ok, thanks.

<snip>
cmp() is not defined anywhere. That's a constraint violation and your
code won't compile. Post compilable code.

cmp is the fourth argument to insertion_sort(), it compiles for me.

 

[evanevankan2]

I'm sorry for my previous reply in which I said 'cmp' is not defined
anywhere. It's defined in the functions definition as a pointer to a
function.

And I am sorry I missed this post before I replied to the first one.

On Jul 13, 5:17 pm, (e-mail address removed) wrote:> I have a question about the warning 'comparison between signed and

Do you want to change your code? It's just a warning, comparing signed
with unsigned doesn't invoke undefined behavior in any way.

No, if the comparison is ok in this case I don't feel the need to
change it.

As I said before, only if the unsigned integers value cannot be
correctly represented by the signed integer type.

Nothing. Though the results, while defined, might be not the ones you
expect, for instance:
10u > -1
is not true. -1 gets promoted to unsigned int, and becomes UINT_MAX.'

I understand. I guess I'll just have to be careful when doing such
comparisons then.

What if previous allocations succeded? Memory leak.

Yeah, I usually don't write code like that that just aborts. I often
see code with functions named xmalloc() that just bails out when
memory allocation fails, don't like that, but this was only for
testing so I decided to do it like that this time.

If you change these to size_t, your second for loop won't work,
specifically the part j >= 0, which is always true, since j would be
size_t, an unsigned integer.
<snip>

I saw that changing 'j' would break the code, that's why I only said
'i' in my first post. Having i and j as different types is pretty ugly
IMHO, so I guess I'll just leave the code as it is.

Thanks for the help!