0

What is the difference between passing an argument to pthread_create as int instead of long?

int pthread_create(pthread_t *restrict thread,
          const pthread_attr_t *restrict attr,
          void *(*start_routine)(void*), void *restrict arg);

I took the code from POSIX Threads Programming and it works only declaring the function argument as long

void *PrintHello(void *threadid) {
    long tid;
    tid = (long)threadid;
    printf("Hello World! It's me, thread #%ld!\n", tid);
    pthread_exit(NULL);
} 

int main(int argc, char *argv[]) {
    pthread_t threads[NUM_THREADS];
    int rc;
    long t;
    for(t=0;t<NUM_THREADS;t++){
        printf("In main: creating thread %ld\n", t);
        rc = pthread_create(&threads[t], NULL, PrintHello, (void *)t);
        if (rc){
            printf("ERROR; return code from pthread_create() is %d\n", rc);
            exit(-1);
        }
    }

    /* Last thing that main() should do */
    pthread_exit(NULL);
}
federico
  • 69
  • 1
  • 10
  • 1
    strictly speaking, I don't believe one should be casting either `int` or `long` to/from `void *`. If you want to convert a `void *` to an integral type, you should be using `intptr_t` or `uintptr_t`. – Christian Gibbons Sep 05 '19 at 19:12
  • Although even the `intptr_t` and `uintptr_t` types are specified to be able to convert from a valid `void` pointer and back. Nothing about taking just any value and creating a `void` pointer from it. Really, casting like this is lazy hack and probably amounts to undefined behavior. – Christian Gibbons Sep 05 '19 at 19:40
  • 1
    @ChristianGibbons It is not undefined behavior. Per [**https://port70.net/~nsz/c/c11/n1570.html#6.3.2.3p5**, paragraphs 5 and 6](https://port70.net/~nsz/c/c11/n1570.html#6.3.2.3p5): "5 An integer may be converted to any pointer type. Except as previously specified, **the result is implementation-defined**, might not be correctly aligned, might not point to an entity of the referenced type, and might be a trap representation." – Andrew Henle Sep 05 '19 at 19:57
  • (cont, para 6) "6 Any pointer type may be converted to an integer type. Except as previously specified, **the result is implementation-defined**. If the result cannot be represented in the integer type, the behavior is undefined. The result need not be in the range of values of any integer type. " – Andrew Henle Sep 05 '19 at 19:57
  • 1
    @AndrewHenle Ah, thank you for that reference. – Christian Gibbons Sep 05 '19 at 20:13
  • @ChristianGibbons You're welcome. Converting `int` values to and from `void *` under POSIX (the use of `pthread_create()` being informative...), is generally a hack that is going to "work" as long as the `int` values fit into a `void *` value, barring platform-specific limitations. See https://stackoverflow.com/questions/7822904/is-it-always-safe-to-convert-an-integer-value-to-void-and-back-again-in-posix But yeah, it's a hack. ;-) – Andrew Henle Sep 05 '19 at 20:20

3 Answers3

3

In this case, the issue has nothing to do with threads, but with the conversion between void * and integer types. 'c' allows you to freely convert across these types, but you have to pay attention to the sizes of the objects that you convert.

For example, on x86 in 64-bit mode the size of the void* is 64 bit (8 bytes). It is the same as the size of long int. However, size of int is 32 bits (4 bytes).

So, conversion of an int to (void*) will sign-extend int to 64 bits and assign it to void. Compiler gives you a warning, because sizes in the conversion are different but there is o violation of the standard here.

Conversion back to int from void * will also generate a warning. This conversion will loose upper 32 bits. But it will convert it correctly to int, if the void * itself was assigned from an int. However, if you try to convert it to long and the original int was negative, you will get a completely different picture, it will not be the same as the original int, all upper 32 bits will be a result of the original sign extension.

To make the picture more interesting, it will work differently on different platforms. For example, on 32-bit platforms size of long int could be the same as the size of int and the size of void*.

Potentially size of long int usually matches the size of void*. At least it does for a few platforms. Therefore this conversion is the safest one.

So, the best solution is to avoid such conversions when you write a cross-platform code.

Serge
  • 11,616
  • 3
  • 18
  • 28
0

Your are treating the argument as a long rather than a pointer to long. This means you are depending on long and void * to have the same data size. If int has a different size this could explain why it goes wrong. In any case you should pass &t in the argument and extract it accordingly as *threadid:

void *PrintHello(void *threadid) {
    long tid;
    tid = *((long *)threadid); 
    printf("Hello World! It's me, thread #%ld!\n", tid);
    pthread_exit(NULL);
} 

int main(int argc, char *argv[]) {
    pthread_t threads[NUM_THREADS];
    static long tid[NUM_THREADS];
    int rc;
    long t;
    for(t=0; t<NUM_THREADS; t++) {
        printf("In main: creating thread %ld\n", t);
        tid[t] = t;
        rc = pthread_create(&threads[t], NULL, PrintHello, (void *)&tid[t]);
        if(rc) {
            printf("ERROR; return code from pthread_create() is %d\n", rc);
            exit(-1);
        }
    }

    /* Last thing that main() should do */
    pthread_exit(NULL);
}

Update: Store thread IDs in array tid.

nielsen
  • 5,641
  • 10
  • 27
  • I don't believe the size of `int` vs `long` should be an issue since there's no pointer dereferencing happening. – Christian Gibbons Sep 05 '19 at 19:11
  • Have you tried running your code? I don't think it will produce the expected result. – manish ma Sep 05 '19 at 19:12
  • The code works, you don’t need to use &. The problem is using int instead of long – federico Sep 05 '19 at 19:13
  • @federico what kind of problem have you discovered? if you pass something as `(void *)int` and then cast it to `(void*)long` you should expect to have trash in the upper part of the `long`. Cast it to `int` instead. – Serge Sep 05 '19 at 19:19
  • All the threads get passed a pointer to the same variable. This makes no sense when using `t` as thread ID – HAL9000 Sep 05 '19 at 19:24
  • @Serge Actually it's not an error, it's a warning, but I want to know why is throwing that. Pointer to integer of different size – federico Sep 05 '19 at 19:26
  • @Christian Gibbons You are right it should not be an issue. My main point is that casting an integer (or long) to the `void *` argument is risky so it is much better to pass a pointer to whichever data type is needed. – nielsen Sep 05 '19 at 19:31
  • @federico What is exactly the warning you get with the `int`? – nielsen Sep 05 '19 at 19:32
  • @nielsen Warning: cast to pointer from integer of different size This is not happening when you use long – federico Sep 05 '19 at 19:35
  • Run your code and watch what happens. The main thread may actually advance t a couple of times before the newly created threads print the value stored at t. So I think in this case passing the adress is a bad idea. – manish ma Sep 05 '19 at 19:36
  • @sergeyrar You are right, I updated the code to store the thread IDs in an array. Thanks for noticing this. – nielsen Sep 05 '19 at 19:39
  • @federico Then that _is_ the problem. The `int` type has a different size than `void *` on your machine (you can verify that by printing `sizeof(int)` and `sizeof(void *)`. If you pass pointers as I show above, you can change the type of the `tid` array in `main()` and the `tid`variable in `PrintHello()` to `int`. – nielsen Sep 05 '19 at 19:42
  • After changing `long t` into `long tid[]`, the code works, but it fails to answer the posters question. Neither does it improve the posters code. – HAL9000 Sep 19 '19 at 23:13
  • @HAL9000 That is your subjective opinion. I do not agree, but this is not a discussion forum. The point of my answer is that the problem in the question comes from passing non-pointer data in a pointer-type argument and hence I show how to avoid that. The accepted answer from Serge explains this in more detail and makes the same conclusion. – nielsen Sep 20 '19 at 08:22
0

the following proposed code:

  1. cleanly compiles
  2. performs the desired functionality
  3. cleans up after itself, even when an error occurs
  4. properly (using perror()) outputs the OPs error message AND the text reason the system thinks the error occurred to stderr.
  5. uses the signature: int main( void ) so no messages from the compiler about unused parameters.
  6. Since a loop counter cannot be < 0, uses variable type size_t for the loop counter.
  7. properly initializes (to 0) the pthread_t array to 0 so can be used for cleanup operations
  8. Corrects the format specifiers in the calls to printf() to expect a variable with type: size_t
  9. Has the main() function properly wait for the threads to exit before the main() function exits
  10. incorporates a new function: cleanup() to properly cleanup any threads before exiting the program.
  11. uses appropriate horizontal spacing and vertical spacing for readability and ease of understanding.

and now, the proposed code:

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

#define NUM_THREADS 20


void *PrintHello( void *arg ) 
{
    size_t threadNum = (size_t)arg;

    printf( "Hello World! It's me, thread #%lu!\n", threadNum );
    pthread_exit( NULL );
} 



void cleanup( pthread_t *threads )
{
    for( size_t i=0; i<NUM_THREADS; i++ )
    {
        if( threads[i] )
        {
            pthread_join( threads[i], NULL );
        }
    }
}


int main( void ) 
{
    pthread_t threads[ NUM_THREADS ] = {0};


    for( size_t t = 0; t < NUM_THREADS; t++ )
    {
        printf( "In main: creating thread %ld\n", t );
        if( pthread_create( &threads[t], NULL, PrintHello, (void *)t ) )
        {
            perror( "pthread_create() failed" );
            cleanup( threads );
            exit( EXIT_FAILURE );
        }
    }

    cleanup( threads );
    /* Last thing that main() should do */
    pthread_exit( NULL );
}

here is the output from a typical run of the proposed code when no errors occur:

In main: creating thread 0
In main: creating thread 1
Hello World! It's me, thread #0!
In main: creating thread 2
In main: creating thread 3
In main: creating thread 4
Hello World! It's me, thread #3!
In main: creating thread 5
Hello World! It's me, thread #4!
In main: creating thread 6
In main: creating thread 7
Hello World! It's me, thread #6!
Hello World! It's me, thread #5!
In main: creating thread 8
Hello World! It's me, thread #7!
In main: creating thread 9
Hello World! It's me, thread #8!
In main: creating thread 10
In main: creating thread 11
In main: creating thread 12
Hello World! It's me, thread #10!
Hello World! It's me, thread #9!
Hello World! It's me, thread #11!
In main: creating thread 13
In main: creating thread 14
Hello World! It's me, thread #13!
In main: creating thread 15
Hello World! It's me, thread #12!
Hello World! It's me, thread #14!
In main: creating thread 16
Hello World! It's me, thread #15!
In main: creating thread 17
In main: creating thread 18
In main: creating thread 19
Hello World! It's me, thread #19!
Hello World! It's me, thread #16!
Hello World! It's me, thread #17!
Hello World! It's me, thread #18!
Hello World! It's me, thread #1!
Hello World! It's me, thread #2!
user3629249
  • 16,402
  • 1
  • 16
  • 17