Search code examples
catomicvolatile

Proper use of 'volatile' in this case (C)?


I have a structure that holds several pointers. These pointers can be changed by several different threads. These threads update the struct by changing the pointer so that it points at another memory location, they never change the value that is pointed to. Based on my understanding of volatile, it makes sense to declare these pointers in the struct as volatile.

The threads that read/update these pointers work like this:

  • copy pointer, only use our copy, that way if original changes we don't suddenly use the new one halfway through the process.
  • create a new pointer based on the value of what our copy points at.
  • use atomic compare+swap to replace the old pointer with the new one, unless another thread has already updated it.

The issue I am hitting is that when I make my copy of the pointer, I get a warning:

warning: initialization discards ‘volatile’ qualifier from pointer target type

The compiled code seems to work fine, but the warning bothers me. Am I using volatile incorrectly? Assuming my use of volatile is fine, how do I remove the warning? I don't want to slap volatile onto the declaration of the copy pointer, no other threads will ever update our copy, so it really is not volatile. Its only volatile when reading/writing the struct.

Here is some demo code that simply demonstrates my question, the actual code I am working with is much better, but also too big to post. There is an obvious memory leak, ignore it for now my real use-case properly tracks and manages the memory. I am only concerned with 'volatile' for this question, I am not looking for bugs in my demo to be solved.

gcc -std=gnu99 -pthread test.c && ./a.out
test.c: In function ‘the_thread’:
test.c:22:25: warning: initialization discards ‘volatile’ qualifier from pointer target type [enabled by default]

The code:

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

typedef struct {
    volatile int *i;
} thing;

void init( thing *t ) {
    t->i = malloc( sizeof( int ));
    *(t->i) = 1;
}

void *the_thread( void *args ) {
    thing *t = args;

    for ( int i = 0; i < 100; i++ ) {
        // This generates the 'volatile' warning. But I don't want to make
        // *copy volatile since no other threads will ever change 'copy'.
        // Is this incorrect usage, or do I just need to do something to remove
        // the warning?
        int *copy = t->i;

        int *new = malloc( sizeof( int ));
        *new = (*copy) + 1;

        // Glaring memory leak as old x becomes unreachable, this is a demo,
        // the real program this is for has the issue solved.
        // We do not care if it succeeds or fails to swap for this demo.
        __sync_bool_compare_and_swap( &(t->i), copy, new );
    }
}

int main() {
    thing t;
    init( &t );

    pthread_t a;
    pthread_t b;

    pthread_create( &a, NULL, the_thread, &t );
    pthread_create( &b, NULL, the_thread, &t );

    pthread_join( a, NULL );
    pthread_join( b, NULL );

    return 0;
}

Solution

  • This is because volatile int * doesn't mean "volatile pointer to int", it means "pointer to volatile int". Compare const char *a = "foo";, that's a pointer to constant character data, not a constant pointer.

    So in your case, I believe it's the thing pointer that should be volatile, since the fields in it can change "randomly" (from one thread's point of view). You can (as you say in a comment) also move the volatile to make it int * volatile i; in the struct.

    You can (of course) always use cdecl to get instant help figuring these things out, too.