Here is my code,

#include <time.h>
#include <pthread.h>
#include <string.h>
#include <inttypes.h>
#include <unistd.h>

unsigned sleep(unsigned sec) ;

void get_time(char *buf) {  
    time_t t = time(NULL) ;
    struct tm tm = *localtime(&t) ;
    sprintf(buf,"d:d:d", tm.tm_hour, tm.tm_min, tm.tm_sec) ;
}

void *calculateSize(uint64_t size, char *buf) {   
    if (size < 1024ULL){
        sprintf(buf, "%.1f %s", (float) size, "B") ;
        return NULL ;
        }    
    else if (size < (1024 * 1024)) {
        sprintf(buf, "%.1f %s", (float) size/1024ULL, "KiB") ;
        return NULL ;
    } 
    else if (size > (1024 * 1024)) {
        sprintf(buf, "%.1f %s", (float) size/(1024ULL * 1024ULL), "MiB") ;
        return NULL ;
    } 
    strcpy(buf, "0") ;
    return NULL ; 
}

void * run(char *interface, char * download_buf, char * upload_buf) {
    unsigned long rdiff,tdiff ;
    unsigned long rx_old,rx_new ;
    unsigned long tx_old,tx_new ;
    char buf[10] ;
    char rx_file[512] ;
    char tx_file[512] ;
    sprintf(rx_file, "/sys/class/net/%s/statistics/rx_bytes", interface) ;
    sprintf(tx_file, "/sys/class/net/%s/statistics/tx_bytes", interface) ;
    
    FILE *rf = fopen(rx_file,"r") ;
    FILE *tf = fopen(tx_file,"r") ;
    
    if (rf != NULL && tf != NULL) {
        fscanf(rf,"%lu", &rx_old) ;
        fscanf(tf,"%lu", &tx_old) ;
        
        fclose(rf) ;
        fclose(tf) ;
    }
    else {
        return NULL ;   
    }
    
    sleep(1) ;
    
    rf = fopen(rx_file,"r") ;
    tf = fopen(tx_file,"r") ;
    
    if (rf != NULL && tf != NULL) { 
        fscanf(rf,"%lu", &rx_new) ;
        fscanf(tf,"%lu", &tx_new) ;
    
        rdiff = rx_new - rx_old ;
        tdiff = tx_new - tx_old ;                       
                
        fclose(rf) ;
        fclose(tf) ;
    }
        
    else {
        return NULL ;   
    }
    calculateSize(rdiff,buf) ;
    strcpy(download_buf,buf) ;
    calculateSize(tdiff,buf) ;
    strcpy(upload_buf,buf) ;
    return NULL ;
}

void *net_speed(void *thread_speed_args ) {
        char* iface = *(char **)thread_speed_args ;
        char carrier_file[512] ;
        sprintf(carrier_file,"/sys/class/net/%s/carrier", iface) ;
        printf("Reading from %s\n", carrier_file) ;
        while(1) {
                if( access( carrier_file, F_OK ) == 0 ) {
                    run(iface, ((char **)thread_speed_args)[1], ((char **)thread_speed_args)[2]) ;
                    } 
            else {
            sprintf(((char **)thread_speed_args)[1],"000 B") ;
            sprintf(((char **)thread_speed_args)[2],"000 B") ;
                sleep(1) ;
                    }
        }
        return NULL ;    
}

int main(int argc, char *argv[]) {
    char time_buf[10] ; //hh:mm:ss : 8 char   1 null terminator char
    char download_buf[8],upload_buf[8] ;
    char* thread_speed_args[3] = { argv[1], download_buf, upload_buf } ;
    
    pthread_t thread_speed ;
    pthread_create(&thread_speed, NULL, net_speed, thread_speed_args) ;
    pthread_detach(thread_speed) ;
    
    while(1){
        get_time(time_buf) ;
        printf("Down:%s Up:%s %s\n", thread_speed_args[1], thread_speed_args[2], time_buf) ;
        fflush(stdout) ;
        sleep(1) ;
    }
}

I am using a thread to continuously monitor my wireless data transfer rate on an interface, I am also printing the time along with the transfer rate. I am just experimenting with threads, forgive me if my programming logic is bad.

I notice that when I do a speed test the output or print becomes all messed up like below

Down:0.0 B Up:0.0 B 23:49:03
Down:0.0 B Up:0.0 B 23:49:04
Down:0.0 B Up:0.0 B 23:49:05
Down:17.0 KiB9.2 KiB Up:9.2 KiB 23:49:06
Down:5.3 KiB Up:6.5 KiB 23:49:07
Down:3.4 KiB Up:4.1 KiB 23:49:08
Down:400.6 Ki20.3 KiB23:49:09 Up:20.3 KiB23:49:09 23:49:09
Down:918.6 Ki49.6 KiB23:49:10 Up:49.6 KiB23:49:10 23:49:10
Down:912.8 Ki53.5 KiB23:49:11 Up:53.5 KiB23:49:11 23:49:11
Down:959.2 Ki32.2 KiB23:49:12 Up:32.2 KiB23:49:12 23:49:12
Down:711.5 Ki33.8 KiB23:49:13 Up:33.8 KiB23:49:13 23:49:13

See the last few lines?

Could anyone tell me what is happening here? How do I correct it?

CodePudding user response：

This is happening because you're accessing a common resource in multiple threads simultaneously - in this case, the STDOUT handle. You'll need to synchronize your threads when outputting the information to the console - the easiest way is using mutexes. You can read more here.

CodePudding user response：

While your threads lack any kind of synchonization (which will randomly garble your output in some rare cases) this isn't even your problem.

Here is your problem:

char download_buf[8],upload_buf[8] ;

char buf[10] ;
strcpy(download_buf,buf) ;
strcpy(upload_buf,buf) ;

You are copying an up to 9 character string into a 7 char string so that overflows the buffer. And since the buffers happen to be next to each other on the stack the second strcpy will overwrite the 0 terminator of the first string. Same with the time_buf overwriting the 0 terminator of the second string.

Overall a whole lot of undefined behavior due to buffer overflows.