1

How do I initialize array of structures with values? So, I have structure color with rgb values.

struct color{
    GLfloat r;
    GLfloat g;
    GLfloat b;
}

and trying to initialize it with 1.0f.

color* cArray = (color*) malloc(w*h*sizeof(color));
memset(&cArray, 1.0, sizeof color);

But instead of correct work I get segmentation fault on cArray[0]. What do I miss?

mersinvald
  • 19
  • 2
  • `memset` used to set all values either to `0` or `-1`. Use a loop to set all values of an array to any values other than `0` or `-1`. – haccks Jun 05 '15 at 21:21
  • Note that `color` cannot be use as such in C (that would be C++). `struct` has its own namespace, so you have to either `typedef` the struct or use `struct color` in the definition of the pointer. – too honest for this site Jun 05 '15 at 23:05

4 Answers4

3

Note: Do not cast void * (result of malloc()) to other pointers and always check the result of malloc()& friends.

Memset takes an unsigned char (most likely 8 bits) and treats the area passed as an array of char. It has no idea of structs, arrays etc. Just of a 1-dimensional array of char. (as @chqrlie noted, memset() actually takes an int; this is, however, internally converted to unsigned char).

The segfault is because you pass the address of cArray which is a pointer to the actual array, not its value which would be the array itself.

For float, memset() most likely only makes sense setting it to all-0:

memset(cArray, 0, sizeof(struct color) * w * h);

Note: memset has no idea on data types. It just takes a pointer to a block of memory and a count and stores the value to that area. It is your responsibility all arguments are valid!

Note that writing the memory area with 0 is actually float zero (0.0, actually +0.0), as that has all bits cleared. That was a briliant intention by the guys who dsigned the encoding for floats.

As you allocate right before, you can combine this using calloc(). That will allocate and clear the memory area of the array. However, if you intend to set all values explicitly anyway, you better stick with malloc().

If you want to set other float values, you have to write your own loop. However, you can threat the array as 1-dimensional for that.

for ( size_t i = 0 ; i < w * h ; i++ )
    cArray[i] = (struct color){ .r = 1.0, .g = 1.0, .b = 1.0 };

That uses a compound literal. Alternatively you can also set the fields seperately.

If you are upt for speed, the compount-literal approach might be the fastest. This way ,the compiler might very well load al values into three registers and store them directly into memory using store-multiple. However it might do, I would bet the compiler will recognize that pattern and optimize as hell, as it is commonly used for very large loops.

too honest for this site
  • 12,050
  • 4
  • 30
  • 52
2

You can't use memset() to set float values.

The memset() function is used for bytes, so using it for floats is not allowed, you need to explicitly initialize each member

This is memset()'s signature

void *memset(void *s, int c, size_t n);

although it expects int1 for it's second parameter, the standard says

7.24.6 Miscellaneous functions

7.24.6.1 The memset function

  1. The memset function copies the value of c (converted to an unsigned char) into each of the first n characters of the object pointed to by s.

"converted to an unsigned char" so it sets bytes.

Also take in consideration the following:

  1. You don't need to cast the return value of malloc(), and it's better if you don't do it.

  2. Always check that malloc() returned non-NULL, specially when allocating a lot of memory.


1As you can see you can't pass float.

Community
  • 1
  • 1
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
  • Ok, i'll notice. So what can I use to set this array? I has more then 50000 vertices in vertex array, so for-loop is too expensive for per-frame updating – mersinvald Jun 05 '15 at 21:17
  • `memset()` does loop after all, so I don't think there would be another way. – Iharob Al Asimi Jun 05 '15 at 21:18
  • @mersinvald: The `for` loop can be optimized to the max by the compiler. See this page: http://gcc.godbolt.org/# . You can also keep a copy of a pre-initialized array and use `memcpy` to initialize the array. – chqrlie Jun 05 '15 at 21:29
  • @chqrlie yes, one like the one in @[Olaf](http://stackoverflow.com/a/30676053/1983495)'s answer. – Iharob Al Asimi Jun 05 '15 at 21:32
  • @iharob: not exactly, the OP could keep a full initialized frame and use `memcpy(cArray, cStaticFrame, w * h * sizeof(color));` to initialize it. It might be more or less efficient depending on his configuration. Benchmarking is required. – chqrlie Jun 05 '15 at 21:44
  • Yes initialized statically with `{ .r = 1.0, .g = 1.0, .b = 1.0 }`. – Iharob Al Asimi Jun 05 '15 at 21:45
  • @iharob: No, initialized with a loop because `w` and `h` are variables. – chqrlie Jun 05 '15 at 21:46
  • @chqrlie: `memcpy()` will likely be slower than explicit initializing and use up cache. – too honest for this site Jun 05 '15 at 22:03
  • 1
    It's interesting that someone would downvote this answer, I wish they leave a comment, because I can't really see anything wrong with it, but it seems that lately someone is just downvoting **my** posts, which is a stupid thing. I have had more than 100 rep points reverse because of serial downvoting. – Iharob Al Asimi Jun 05 '15 at 22:12
2

There are multiple problems with your code:

You allocate a matrix w x h of color structures:

color *cArray = (color*) malloc(w * h * sizeof(color));

The cast is not necessary in C, but opinions differ about the recommended alternatives. A safer version would be:

color *cArray = malloc(w * h * sizeof(*cArray));

More importantly, our call to memset is incorrect in multiple ways:

memset(&cArray, 1.0, sizeof color);
  1. You pass the address of the pointer instead of the value of the pointer cArray.
  2. You only pass the size of a single color structure. This size exceeds the size of the pointer, hence the crash. If you intended to set the whole array you should have store the size into a temporary variable.
  3. You pass a double instead of an int as the value to set. You cannot initialize an array of double values with memset: this function initializes a block of memory by setting all bytes to the same value. Passing 0 would initialize double values correctly if your system uses IEEE-754 representation for floating point values, otherwise it might invoke undefined behaviour.

You must initialize the matrix with a loop:

for (size_t i = 0, n = w * h; i < n; i++) {
    cArray[i].r = cArray[i].g = cArray[i].b = 1.0;
}

This loop can be optimized by modern compilers. I suggest you play with this very interesting tool: http://gcc.godbolt.org/# .

You can also keep a copy of a pre-initialized array and use memcpy to initialize the allocated array.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
-2

First, colors are (in your case) 3 bytes, not 3 floats.

then this code:

struct color{
    GLfloat r;
    GLfloat g;
    GLfloat b;
}

and trying to initialize it with 1.0f.

color* cArray = (color*) malloc(w*h*sizeof(color));
memset(&cArray, 1.0, sizeof color);

will cause a compiler to raise several warnings:

due to the compiler will not knowing what a 'color' is due to:

syntax errors in the struct definition
and references to the struct are missing the 'struct' modifier.

when calling malloc() (and family of functions) always check (!=NULL) the returned value to assure the operation was successful.

In C, do not cast the returned value from malloc() (and family of functions)

notice that a struct definition ends with a ';'

suggest using:

struct color
{
    char r;
    char g;
    char b;
};

struct color* cArray = NULL;
if( NULL == (cArray =malloc(w*h*sizeof(struct color))))
{ // then malloc failed
    perror( "malloc for struct color array failed" );
    exit( EXIT_FAILURE )
}

// implied else, malloc successful

memset(&cArray, 1, w*h*(sizeof (struct color) );
user3629249
  • 16,402
  • 1
  • 16
  • 17