0

I have a function that counts the number of occurences of a string in a char array. Calling this function normally with findword(copy, "polar") works perfectly fine and prints an int that's the number of times the string "polar" occurs in the char array "copy". Calling the function via a pthread however gives me compilation issues and I don't know why. This is my first time implementing multithreading.

Here is the function I want to call:

void findword(char *str, string word)
{
    char *p;
    vector<string> a;
  
    p = strtok(str, " ");
    while (p != NULL)
    {
        a.push_back(p);
        p = strtok(NULL, " ");
    }

    int c = 0;
    for (int i = 0; i <= a.size(); i++)
  
        
        if (word == a[i])
            c++;
    printf("%d", c);
}

And here is the thread I'm trying to create that is supposed to call the function:

struct findwordargs {
  char *str;
  string word;
};

struct findwordargs firstwordArguments;
firstwordArguments.str = copy;
firstwordArguments.word = "polar";

pthread_t thread_id = 1;
pthread_create(&thread_id, NULL, findword, (void *)(&firstwordArguments));
pthread_join(thread_id, NULL);


whenever I compile using g++ and the -pthread flag I get this compile error:


error: invalid conversion from ‘int (*)(char*, std::string)’ {aka ‘int (*)(char*, std::__cxx11::basic_string<char>)’} to ‘void* (*)(void*)’ [-fpermissive]
  101 | pthread_create(&thread_id, NULL, findword, (void *)(&firstwordArguments));

All the necessary header files are included, thank you for the help.

François Andrieux
  • 28,148
  • 6
  • 56
  • 87
kikikey
  • 105
  • 1
  • 8
  • 2
    This looks like C code except that it uses `std::string` (based on the error message) and `std::vector` which are part of C++. It looks to me like you are confusing C and C++ as the same language and using examples from both. In C++ you should use `std::thread` or `std::async` instead of `pthread` directly. In C you can't use `std::string` because it doesn't exist. I removed the C tag because while this is obviously not code meant for C++, it could at least compile as C++. – François Andrieux Sep 20 '21 at 15:36
  • 1
    When you use `g++`, are you using `-std=c++98`? The C++98 standard is not multithreaded, and adding multithreading (such as POSIX threads) has caveats. If you are using modern C++ (C++11 or later), it has multithreading facilities as part of the language. (The modern C++ threading on some platforms may be implemented in terms of POSIX threads. But those shouldn't be used directly, instead use the C++ abstractions.) – Eljay Sep 20 '21 at 15:40
  • 1
    using pthreads requires some dirty manual work, because you can only pass a `void* (*)(void*)` function pointer, thats what the error message is trying to tell you. You cannot pass a function pointer to a `void(char *, string)` – 463035818_is_not_an_ai Sep 20 '21 at 16:08
  • Where is `copy` declared? Will it outlive the thread? Hint: It must since you use a pointer to it in the thread. – Ted Lyngmo Sep 20 '21 at 16:37
  • If you share data between threads you must keep it alive. Hint, if it is not a prio clear which thread lives longest then threads should have shared ownership of data. – Pepijn Kramer Sep 20 '21 at 17:07

2 Answers2

2

Your findword() function does not match the signature that pthread_create() requires:

void *(*start_routine)(void *) 

Try this instead:

struct findwordargs
{
    char *str;
    std::string word;
};

void* findword(void *param)
{
    findwordargs *args = static_cast<findwordargs*>(param);
    std::vector<std::string> a;

    char *p = strtok(args->str, " ");
    while (p) {
        a.push_back(p);
        p = strtok(NULL, " ");
    }
    int c = 0;
    for (size_t i = 0; i < a.size(); ++i) {
        if (args->word == a[i])
            c++;
    }
    printf("%d", c);

    /* alternatively, using more C++-ish routines:
    std::istringstream iss(args->str);
    std::string word;
    while (iss >> word) a.push_back(word);
    std::cout << std::count(a.begin(), a.end(), args->word);
    */

    return NULL;
}

...

findwordargs firstwordArguments;
firstwordArguments.str = copy;
firstwordArguments.word = "polar";

pthread_t thread_id = 1;
pthread_create(&thread_id, NULL, findword, &firstwordArguments);
pthread_join(thread_id, NULL); 

That being said, there is no point in creating a thread just to immediately join it. That blocks the calling thread until the spawned thread exits, which is the same effect as simply calling the function directly, but without the overhead of context switching between threads:

void findword(const std::string &str, const std::string &word)
{
    std::vector<std::string> a;
    std::istringstream iss(str);
    std::string word;
    while (iss >> word) a.push_back(word);
    std::cout << std::count(a.begin(), a.end(), word);
}

...

findword(copy, "polar");
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • I tried your solution with thread creation but I just get this error instead: In function ‘void* findword(void*)’: error: conversion from ‘findwordargs*’ to non-scalar type ‘findwordargs’ requested 25 | findwordargs args = static_cast(param); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: base operand of ‘->’ has non-pointer type ‘findwordargs’ 28 | char *p = strtok(args->str, " "); | ^~ error: base operand of ‘->’ has non-pointer type ‘findwordargs’ 35 | if (args->word == a[i]) – kikikey Sep 20 '21 at 17:45
  • @kikikey sorry, that was a typo on my part. `findwordargs args = ...` should have been `findwordargs *args = ...`. It is fixed now. – Remy Lebeau Sep 20 '21 at 18:22
-1

Is there a reason you're using pthreads instead of std::thread? If so, then ignore me. This is what I would do:

std::thread myThread([=](){ findword(copy, "polar"); });
myThread.join();

-or-

myThread.detach(); // If appropriate.

This depends on something resembling modern C++.

Joseph Larson
  • 8,530
  • 1
  • 19
  • 36
  • 3
    `myThread.detach()` is so rarely appropriate that it shouldn’t even be mentioned in advice to beginners. – Pete Becker Sep 20 '21 at 16:00
  • @PeteBecker Do you have a definitive source for that? – Joseph Larson Sep 20 '21 at 16:33
  • 3
    Yes; me. Seriously: every use of `detach()` that I've seen on Stack Overflow has been wrong. Beginners see it as an alternative to `join()` when they can't figure out where in their code to put a `join()`, rather than seeing it as a design decision. – Pete Becker Sep 20 '21 at 17:35