Concatenating a string and numbers in C-CodePudding

I have a program that has to count the number of duplicate characters in a string. For example "aaabcc" should return "a3b1c2" and "aaabcccc..a" should return "a3b1c4.2a1". I am currently using sprintf to concatenate the string and numbers like this: sprintf(s, "%s%c%d", s, prev, count);, where s is the string containing the result. But that causes an error since I am using "s" as an input and also as the destination. Is there some way around this?

This is my code right now:

    char *s = (char *)malloc(sizeof(char) * 100);
    char prev = argv[1][0];
    if(isdigit(prev)!=0){
        printf("%s","ERROR");
        return 0;
    }
    
    int count = 1;
    for(int i=1; i<strlen(argv[1]); i  ){
        if(isdigit(argv[1][i])!=0){
            printf("%s","ERROR");
            return 0;
        }
        
        //check if same as previous letter
        if(prev==argv[1][i]){
            count  ;
        }else{
           //add letter and count to string
            //problem
            sprintf(s, "%s%c%d", s, prev, count);
            
            count = 1;
        }
        
        //update prev
        prev=argv[1][i];
    }
    
    //add it to string
    //problem
    sprintf(s, "%s%c%d", s, prev, count);
    
    //check if result is smaller than input
    if(strlen(s) > strlen(argv[1])){
        printf("%s\n", argv[1]);
    }else{
        printf("%s\n", s);
    }
    free(s);

CodePudding user response：

Use a different buffer for sprintf(), then concatenate to s with strcat().

Make sure you initialize s to an empty string after allocating it.

Instead of allocating a hard-coded size for s, allocate a string big enough for the worst case (every character repeated just once, so it's a1b1c1...).

temp can be a short, fixed-size string, since it just has to hold the length of one run.

char *s = malloc(strlen(argv[1]) * 2   1);
s[0] = '\0';
char temp[20];
char prev = argv[1][0];
if(isdigit(prev)!=0){
    printf("%s","ERROR");
    return 0;
}
    
int count = 1;
for(int i=1; i<strlen(argv[1]); i  ){
    if(isdigit(argv[1][i])!=0){
        printf("%s","ERROR");
        return 0;
    }
        
    //check if same as previous letter
    if(prev==argv[1][i]){
        count  ;
    }else{
        //add letter and count to string
        sprintf(temp, "%c%d", prev, count);
        strcat(s, temp);
            
        count = 1;
    }
        
    //update prev
    prev=argv[1][i];
}
    
//add it to string
sprintf(temp, "%s%c%d", prev, count);
strcat(s, temp);
    
//check if result is smaller than input
if(strlen(s) > strlen(argv[1])){
    printf("%s\n", argv[1]);
}else{
    printf("%s\n", s);
}
free(s);

CodePudding user response：

The advice in the comments and other answers are giving you good guidance on how manage the strings that you are dynamically allocating with malloc. But you can simplify your entire program without ever allocating a string and without using sprintf.

Consider this:

int lastChar = '\0';
int count = 0;
const char* arg = argv[1];
int isError = 0;

if (argc < 2) {
    isError = 1;
}

while (*arg & !isError) {     // scan argv[1] for digits
    isError = isdigit(*arg);
    arg  ;
}
arg = argv[1];                // reset arg back to beginning of argv[1]

while (*arg && !isError) { 
    if (lastChar == *arg) {
        count  ;
    }
    else {
        if (count > 0) {
            printf("%c%d", lastChar, count);
        }
        lastChar = *arg;
        count = 1;
    }
    arg  ;
}

// print the last character being tracked or error message
if (isError) {
    printf("ERROR");
}
else if (count > 0) {
    printf("%c%d", lastChar, count);
}
printf("\n");