1

Suppose that we have the following struct definition in a C file:

typedef struct {
    char *name;
    int id;
    int last_cpu;
} thread_t;

(this is for a simulation I'm writing to play with different scheduling algorithms for my OS class). Every time I create a new thread_t struct, how do I deal with the fact that one of the variables is a char*? For example, I have the following method in my code:

thread_t **get_starting_thread_list() {
    thread_t **threads = (thread_t **) malloc(MAX_NUM_THREADS * sizeof(thread *));
    int next_id = 0;

    for (int th_num = 0; th_num < MAX_NUM_THREADS; th_num++) {
        thread_t *new_thread = (thread_t *) malloc(sizeof(thread_t));
        new_thread->name = "New thread";
        new_thread->id = ++next_id;
        new_thread->last_cpu = 0;
        threads[th_num] = new_thread;
    }

    return threads;
}

Could I potentially get a Segmentation Fault if the name field of new_thread is ever "too big"? That is, should I be allocating additional memory for the thread name before assigning it? I've tried running this and it seems to be behaving fine, but I'm afraid that it could potentially break later on.

Ryan
  • 7,621
  • 5
  • 18
  • 31
  • Also Dont cast the value of malloc in C http://stackoverflow.com/a/605858/5339899 and you dont even need to allocate memory for threads just for new_thread – TheQAGuy Oct 05 '15 at 01:14
  • Why don't I need to allocate memory for `threads`? Since `'threads` is what this function is returning, couldn't I get a seg fault if I don't allocate memory for it? – Ryan Oct 05 '15 at 01:17
  • Note: A nice simplification to `thread_t **threads = (thread_t **) malloc(MAX_NUM_THREADS * sizeof(thread *));` is `thread_t **threads = malloc(sizeof *threads * MAX_NUM_THREADS);`. Easier to maintain and less likely to code wrong. – chux - Reinstate Monica Oct 05 '15 at 01:41
  • when calling malloc() do not cast the returned value, because the returned value is a `void *` so can be assigned to any pointer. Always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Oct 06 '15 at 06:06

4 Answers4

1

Could I potentially get a Segmentation Fault if the name field of new_thread is ever "too big"?

No, you cannot get a segfault there, no matter how long the strin may be, because the memory for string literal is allocated statically, and is never copied.

That is, should I be allocating additional memory for the thread name before assigning it?

Not unless you plan to make the name changeable. If you would like to keep the name pointing to the string literal, your code is perfectly fine. Here is what to do if you want to use non-literal data for the names:

char buf[32];
sprintf(buf, "New thread %d", next_id);
new_thread->name = malloc(strlen(buf+1));
strcpy(new_thread->name, buf);

Now you need to call free(threads[i]->name) before de-allocating threads

I've tried running this and it seems to be behaving fine, but I'm afraid that it could potentially break later on.

Your code is fine. You can always use a memory profiler, such as valgrind, to check for invalid access.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • What do you mean that "the memory for the string literal is allocated statically"? – Ryan Oct 05 '15 at 01:14
  • @Ryan The compiler places the characters in a special segment in memory where it keeps string literals. Then this segment is loaded in memory at run time, and your program is given a pointer to it. – Sergey Kalinichenko Oct 05 '15 at 01:15
0

In your snippet you are pointing name at a literal string. If you don't intend to change it that will work fine forever. If you wanted to do something like ptr->name[0] = 'X'; You're now trying to modify a fixed string, which I think is undefined behavior (watch the comments for a language lawyer to confirm or deny ;-).

John3136
  • 28,809
  • 4
  • 51
  • 69
0

In your code, the literal string "New thread" is allocated statically, not dynamically. (Meaning that this text exists as soon as your application starts, rather than being dynamically allocated from the heap using functions like malloc().) So this will never be too big or cause a segment fault. That string is in static memory and now your thread_t structures will point to that memory.

However, in this case the value will be the same for each structure. So I doubt this is what your code really looks like.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
0

The struct defines char *name; and no matter what string it is pointed at, the size of the pointer will not change.

So the answer to this question: Could I potentially get a Segmentation Fault if the name field of new_thread is ever "too big"? is NO, the size of the string makes no difference.

However, if you define a literal string, then that string cannot be changed. trying to change the contents of a literal string will cause a seg fault event,.

user3629249
  • 16,402
  • 1
  • 16
  • 17