0
\$\begingroup\$

I want to print a nice loading message with these three fading dots, while the main thread does some heavy IO stuff. This is why I implemented this:

#include <stdio.h>
#include <pthread.h>
#include <unistd.h>
#include <stdbool.h>

static bool            loadingStop;
static pthread_mutex_t loadingMutex;

void *loadingDots(void *message);

int main() {
  loadingStop = false;

  // MUTEX
  pthread_mutex_init(&loadingMutex, NULL);

  // ATTRIBUTE
  pthread_attr_t attr;
  pthread_attr_init(&attr);
  pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

  // THREAD
  pthread_t loadingThread;
  pthread_create(&loadingThread, &attr, loadingDots, (void *)"Loading (main, does something long)");

  pthread_attr_destroy(&attr);

  // SOMETHING TIME CONSUMING 
  usleep(8000000);

  // PLEASE 'LoadingDots' TO STOP
  pthread_mutex_lock(&loadingMutex);
  loadingStop = true;
  pthread_mutex_unlock(&loadingMutex);

  // WAIT FOR IT TO FINISH
  pthread_join(loadingThread, NULL);
  pthread_mutex_destroy(&loadingMutex);

  printf("This is printed after 'LoadingDots' savely quit.\n");

  return 0;
}

void *loadingDots(void *message)
{
  int c = 0;

  while (1) {
    switch (c) {
    case 0:
      printf("\r%s   \r%s", (char *)message, (char*)message);
      break;
    default:
      putchar('.');
      break;
    }

    fflush(stdout);
    usleep(1000000);

    c = (c+1) % 4;

    pthread_mutex_lock(&loadingMutex);
    if (loadingStop) { break; };
    pthread_mutex_unlock(&loadingMutex);
  }

  putchar('\n');
  pthread_exit(NULL);
}
\$\endgroup\$
1
\$\begingroup\$

Mostly minor stuff

  1. Good use of fflush(stdout); to insure output. Something often missed.

  2. Minor: int main() { is not explicitly allowed per the standard. Better to use int main(void) {

  3. Minor: Naked physical quantity. Values like height, weight, time, should have units made clear feet vs meter $327.6 million mistake

    // usleep(8000000);
    #define SOME_us_TIME_CONSUMPTION (8000000 /* us */)
    usleep(SOME_us_TIME_CONSUMPTION);
    
  4. Minor: Avoid printf() when fputs() will do. Avoid % issue.. BTW did you mean "safely" vs "savely"?

    // printf("This is printed after 'LoadingDots' savely quit.\n");
    fputs("This is printed after 'LoadingDots' savely quit.\n", stdout);
    // or 
    puts("This is printed after 'LoadingDots' savely quit.");
    
  5. Long lines: Respect the width for the target audience - in this case code review.

    // pthread_create(&loadingThread, &attr, loadingDots, (void *)"Loading (main, does something long)");
    
  6. vs.

    pthread_create(&loadingThread, &attr, loadingDots, 
        (void *)"Loading (main, does something long)");
    
  7. Consider auto format. The space between bool loadingStop will likely shrink with auto formatting. Insure code is presentable even after auto formatting.

    // looks OK
    static bool loadingStop;
    static pthread_mutex_t loadingMutex;
    
  8. Minor: Not a fan of {} one liners

    // if (loadingStop) { break; };
    if (loadingStop) { 
      break;
    };
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.