Iterating through char* and comparing each char to another char

Question

I'm not well versed in c and I'm having issues with

iterating through a char* character by character
correctly comparing the individual character with another character

given a string like "abcda", I want to count the number of "a"s and return the count

    #include <stdio.h>
    #include <unistd.h>
    #include <string.h>

    int main(int argc, char** argv){
        char* string_arg;
        int counter = 0;
        if(argc == 2){
            for(string_arg = argv[1]; *string_arg != '\0'; string_arg++){
                printf(string_arg);
                printf("\n");
                /*given abcda, this prints
                abcda                    a
                bcda                     b
                cda        but i want    c
                da                       d 
                a                        a */

                if(strcmp(string_arg, "a") == 0){ //syntax + logical error
                    counter++;
                }
            }
         printf(counter);
         }
         else{
             printf("error");
         }
         return(0);
    }

I'm also not supposed to use strlen()

How do I compare one character at a time, correctly?

Side note: `printf(string_arg);` is dangerous. `string_arg` is user input, so it can be arbitrary string, including ones that contain `%`. It should be `printf("%s", string_arg);` or `fputs(string_arg, stdout);`. — MikeCAT, May 09 '20 at 08:55
@MikeCAT oh, I see. I'll be sure to implement that when I print the user input. In this case, they were there just so I could get a visual. Thank you. — Andrew, May 09 '20 at 08:58
@RobertSsupportsMonicaCellio I seem to have left out the c when I wrote it up. Edited. — Andrew, May 09 '20 at 09:00

score 1 · Answer 1 · answered May 09 '20 at 09:00

arg isn't declared. It seems it should be argc.
printf(string_arg); is dangerous because string_arg is an user input, which can contain arbitrary string, which may include %.
strcmp() if for comparing strings. You can use simply == to compare characters.
printf(counter); is also wrong.
} at the end of main function is missing.

example fix:

#include <stdio.h>
#include <unistd.h>
#include <string.h>

int main(int argc, char** argv){
    char* string_arg;
    int counter = 0;
    if(argc == 2){
        for(string_arg = argv[1]; *string_arg != '\0'; string_arg++){
            puts(string_arg);
            /*given abcda, this prints
            abcda                    a
            bcda                     b
            cda        but i want    c
            da                       d 
            a                        a */

            if(*string_arg == 'a'){
                counter++;
            }
        }
        printf("%d", counter);
    }
    else{
        printf("error");
    }
    return(0);
}

`arg` was just a typo by writing the question. Already fixed by OP. — RobertS supports Monica Cellio, May 09 '20 at 09:02
this does address the syntax errors but it doesn't address the logical error of the loop. The comparison should be between "a" and "a" not "abcda" and "a". Even that is not happening on the last iteration of the loop @RobertSsupportsMonicaCellio — Andrew, May 09 '20 at 09:10

RobertS supports Monica Cellio · Accepted Answer · 2020-05-09T09:39:22.473

if (strcmp(string_arg, "a") == 0) { 
     counter++;
}

The call to strcmp is not appropriate in your case, as it compares strings. With this statement you compare a string starting at the element pointed to by string_arg with the string "a", not the character constant 'a'. Note that "a" is equal to 'a'+ '\0'.

Instead, You need to compare *string_arg with 'a':

if (*string_array == 'a') { 
     counter++;
}

puts(string_arg); prints a string. That is not what you want. You want to print only a single character. Use printf("%c", *string_arg); instead to print a character.

Note that something like printf(string_arg); is dangerous. Always use a format specifier: printf("%c", *string_arg);. Reasony why is explained under the following link:

Why is printf with a single argument (without conversion specifiers) deprecated?

This shall be what you want:

#include <stdio.h>
#include <unistd.h>
#include <string.h>

int main (int argc, char** argv) {

    char* string_arg;
    int counter = 0;

    if (argc == 2){

        for (string_arg = argv[1]; *string_arg != '\0'; string_arg++) {

            printf("%c", *string_arg);
            printf("\n");

            if (*string_arg == 'a') { 
                counter++;
            }
        }

        printf("%d times character 'a' encountered.", counter);
     }
     else {
        printf("Error: No second argument at the program invocation!");
     }

     return 0;
}

Thank you this, worked. My vote doesn't show because I have <15 reputation though. As for the printing of the remaining parts, I didn't want that. It was just supposed to be a visualization of the fact that I wanted each loop to only focus on a single character. — Andrew, May 09 '20 at 09:21
@Andrew Yes, I recently noticed what your intention was. I corrected my code already. — RobertS supports Monica Cellio, May 09 '20 at 09:22

Iterating through char* and comparing each char to another char

2 Answers2