Why isn't my program displaying every output? It stops after 2 iterations

Question

An application to read atleast 3 or more command line arguments, where every argument consists of servername and port number (0-1023) pair separated by a colon. You need to parse, extract and display all server names and port numbers.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    #define MAX_RANGE 1023
    #define MAX_LENGTH 30
    #define DELIMITER ":"
    
/*
Validate every word received. It should contain a Server name and a port number in range 0-1023  Use isalpha(), isdigit() to validate 
server name and port numbers
 */

    int validateInputs(char *input){
        
        char *serverName = NULL;
        char *portNumber = NULL;
        char token[MAX_LENGTH];
        int portNo;
        int flag1 = 1;
        int flag2 = 1;
        int i;
        int serverLen;
        int portLen;
        
        strcpy(token,input);
        
        serverName = strtok(token, DELIMITER);
        portNumber = strtok(NULL, DELIMITER);
            
        serverLen = strlen(serverName);
        portLen = strlen(portNumber);
        
        for(i = 0; i < serverLen; i++){
            if(!isalpha(serverName[i])){
                flag1 = 0;
                break;
            }
        }
        
        for(i = 0; i < portLen; i++){
            if(!isdigit(portNumber[i])){
                flag2 = 0;
                break;
            }
        }
        
        if(flag1 && flag2){
            if(atoi(portNumber)>0 && atoi(portNumber)<=MAX_RANGE){
                return (EXIT_SUCCESS);
            }
        }
        return(EXIT_FAILURE);
    }

/*
function to receive a word, to extract and display server name and port number. Should store and return pointer to the server name
*/
    
    char* displayServerPort(char *input){
        
        char token[MAX_LENGTH];
        char *serverName = NULL;
        char *portNumber = NULL;
        char *name = NULL;
        int serverLen;
        
        strcpy(token,input);
        
        serverName = strtok(token, DELIMITER);
        portNumber = strtok(NULL, DELIMITER);
        
        printf("Server Name: %s\t Port No: %s\n", serverName, portNumber);
        
        serverLen = strlen(serverName);
        name = (char*) malloc (serverLen * sizeof(char));
        strcpy(name, serverName);
        
        return name;
    }
    
    int main(int argc, char *argv[]){
        
        int i, value;
        
        char *serverNames[MAX_LENGTH];
        char *name = NULL;
        char str[MAX_LENGTH];
        
        if(argc < 3){
            printf("Less number of arguments! Pass atleast 3 arguments");
            exit(EXIT_FAILURE);     
        }
        
        for(i = 1; i < argc; i++){
            
            value = validateInputs(argv[i]);
            if(value == 1){
                exit(EXIT_FAILURE);
            }
            name = displayServerPort(argv[i]);
            strcpy(serverNames[i],name);
        }
    
        return 0;   
    }

If i give the input as serverA-01 serverB-02 serverC-03 the display function is only working for the 1st two arguments(serverA and serverB). When debugging I can see all the 3 outputs but when running the code only shows 1st two args.

yano · Accepted Answer · 2022-02-22T18:37:14.617

You have two problems with your code:

In displayPortServer, you're not allocating enough space for name. strlen does not include the NUL terminator in the length, but strcpy automatically tacks one on at the end, writing beyond the size of you buffer by one, invoking undefined behavior. You need to malloc strlen + 1 bytes so there's enough room for the terminator:

name = malloc(serverLen + 1);

A couple more points on this:

It's not necessary to cast the return value of malloc
sizeof(char) is defined to be one, so you're safe to omit the multiplication.

strcpy(serverNames[i],name); is incorrect. serverNames is defined as an array of char pointers. However, you never initialize those pointers, so they don't point to any valid memory, and writing data to some random memory address you don't own also invokes undefined behavior. The easiest fix here is simply assign name to that pointer, since you're mallocing and returning that in displayPortServer:

name = displayServerPort(argv[i]);
serverNames[i] = name;

Here is a working demonstration, but as the warning says, you're not actually using serverNames. You should also free all the allocated strings in serverNames when you're done, or make a note that you'll let the OS reclaim the memory when the process terminates.

Furthermore, there may be a problem with your test input. Your code specifies that server names must have alpha characters only, so for a server name like "serverA-01", its invalidated as soon as it sees '-', and your program will exit(EXIT_FAILURE).

Thanks, I figured it out already though. I am having just one more problem I'm getting segmentation fault while concatenating the server names from the array. — PyGeek, Feb 23 '22 at 20:32
@PyGeek Make sure your destination string has enough space to hold the new string plus a NUL terminator. For example, if `strlen(str1)==5` and `strlen(str2)==6` and you do `strcat(str1, str2)`, then `str1` must have at least 12 bytes reserved, 5 + 6 + 1 for the terminator. Also, make sure your destination string is _not_ a string literal. For example, `char* str = "hello"` is a string literal, and modifying it is UB. If neither of those is your problem, ask a new question. — yano, Feb 23 '22 at 22:01

Why isn't my program displaying every output? It stops after 2 iterations

1 Answers1