0

I have a struct like:

struct holder {
    int prio;
    long id;
    char * data;
}

But for some reason, I will be using malloced space for just id and data (not the pointer, the actual data).

Now I'm copying the data from holder to the other space like this:

struct holder * my_elem = next_data();
void * buf_k = kmalloc(sizeof(long) + maxlen * sizeof(char), GFP_KERNEL);
memcpy(buf_k, &(my_elem->id), sizeof(long));
memcpy((void *)((char *)buf_k + sizeof(long) * sizeof(char)), my_elem->data, sizeof(char) * str_len(my_elem->data));

However, the code fragment (void *)((char *)buf_k + sizeof(long) * sizeof(char)) is seemed weird to my collugues. Especially the part sizeof(long) * sizeof(char). Isn't it correct, or is it just a weird way of doing the right thing?

totten
  • 2,769
  • 3
  • 27
  • 41

2 Answers2

1

Multiplying by sizeof (char) is quite pointless, because it is by definition always 1. It never can be anything else. The only case where I would do this is when explicitly multiplying number of elements with an element size, like

long* p = malloc (100 * sizeof (long));
int* q = malloc (100 * sizeof (int));
char* r = malloc (100 * sizeof (char));

Multiplying a strlen () by sizeof (char) looks very, very dubious and I'd suspect that this is just a symptom of some severe misunderstanding. And if you use strlen () in an argument for memcpy, I'd expect a comment why you aren't adding 1 to account for the trailing zero byte.

gnasher729
  • 51,477
  • 5
  • 75
  • 98
  • I wouldn't even do that. I'd do "type * p = malloc(100 * sizeof(*p))" in all three of your examples. No need, if the type needs to be changed for some reason, to touch sizeof's operand. – Rob Jan 28 '15 at 10:52
  • sizeof(char) cannot assumed to be always 1 ( http://stackoverflow.com/questions/2215445/are-there-machines-where-sizeofchar-1 ). If you're writing kernel driver, you must use sizeof(char). I'm not adding trailing zero since I'm reporting the sizeof data to user. And user must properly use this. I'm not sending the all data - at least always. – totten Jan 28 '15 at 10:53
  • 1
    sizeof(char) is defined by every C standard to be 1, totten. It is not however guaranteed that a char will be 8 bits. sizeof(char) would be 1 even for a compiler that supported 27 bit char. (27 is a number I just made up on the spot). – Rob Jan 28 '15 at 10:57
  • Even if it's 1 always, it's not answering my question. Is it weird? – totten Jan 28 '15 at 11:01
  • But if it *is* alway 1, then multiplying with it *is* weird. Don't do it. It's not necessarily a "symptom of some severe misunderstanding", it's just a sign that you don't know that it is always 1. As such it is a bit embarassing, and the code is confusing because everybody thinks "wtf is he doing there!?" before one understands that you didn't know sizeof(char) is always 1. The code becomes almost unreadable. – Peter - Reinstate Monica Jan 28 '15 at 11:21
  • `sizeof(char)` is always 1 and in your link it's also said that. A char is always a byte but not always an octet. What can be different is [`CHAR_BIT`](http://stackoverflow.com/questions/2098149/what-platforms-have-something-other-than-8-bit-char?lq=1) – phuclv Jan 28 '15 at 11:28
1

sizeof(char) is always 1. So normally that's left out as it's redundant.

(void *)((char *)buf_k + sizeof(long)

There is also no need to cast to a void* for memcpy(). Any pointer is implicitly convertible to a void* in C, so that cast is redundant too. So you end up with:

(char *)buf_k + sizeof(long)
nos
  • 223,662
  • 58
  • 417
  • 506