0

My code for finding the frequency of digits in a character array in C is not giving the desired output.

int main() {
    int n;
    scanf("%d",&n);
    char str[n];
    int f[10],i,j,ns;
    gets(str);
    for(i=0;i<10;i++)
    {
        f[i]=0;
        for(j=0;j<strlen(str);j++)
        {
            ns=str[j]-'0';
            if(i==ns)
                f[i]+=1;
        }
    }
    for(i=0;i<10;i++)
        printf("%d ",f[i]);
    return 0;
}

If I enter the string

1wjhue73b38409598

I am getting the output of frequency of (0-9):

1 0 0 2 1 1 0 1 2 2 

Instead of frequency 1 for '1'. Where am I going wrong?

dbush
  • 205,898
  • 23
  • 218
  • 273
Tidem23
  • 13
  • 4
  • It would be much simpler to do something like: `f[str[j] - '0']++` – Fiddling Bits Oct 16 '18 at 18:37
  • 1
    @FiddlingBits yikes. There are letters in that string – Tim Randall Oct 16 '18 at 18:38
  • 2
    @TimRandall Thanks for the catch. How about: `if(isdigit(str[j])) f[str[j] - '0']++;` – Fiddling Bits Oct 16 '18 at 18:40
  • 2
    Never use `gets`. It is deprecated. – klutt Oct 16 '18 at 18:42
  • What value did you enter for 'n' before entering the string? Because that's a classic security vulnerability – Tim Randall Oct 16 '18 at 18:42
  • @TimRandall Didn't know that is possible... where is `str` placed on the heap or miracuously stack ? (my guess heap, but with a weird syntax). it sure aint ANSI C. – Tomer W Oct 16 '18 at 18:45
  • @Broman It's more than deprecated; It's *gone* as of C11. It is no longer part of the language standard library. Any implementations that still provides it do so at their own peril; the library no longer requires it *at all*, and implementations are free (and encouraged) to omit it. – WhozCraig Oct 16 '18 at 18:52
  • @TomerW: Variable length arrays, see [VLA](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) – Tom Kuschel Oct 16 '18 at 19:01
  • @TimRandall when my input for n is 50.the output is something like this 0 0 0 0 0 0 0 0 0 -48 -48 (both -48 and -48 are in different lines and all 0s on the 1st line – Tidem23 Oct 16 '18 at 19:30
  • My question has been superseded by dbush's answer. You wouldn't have had a problem if your code had contained `char str[]="1wjhue73b38409598";` instead of the `gets` stuff. – Tim Randall Oct 16 '18 at 19:34
  • But when i am not entering any value of n ,I am getting the output as mentioned in my question. and entering giving a value to n gives me ( 0 0 0..).so how is the former method giving a more accurate output(except for frequency of first element)? – Tidem23 Oct 16 '18 at 19:39
  • I don't understand what you're saying. Or rather, why you're saying it. We've established (below) the problems with your code as written above. I don't think there's any benefit to discussing exactly what it does when it gets specific inputs, because we already know what's wrong with it – Tim Randall Oct 16 '18 at 19:42
  • @Tidem23 It has to do with mixing `scanf` and `gets`. See the update to my answer. – dbush Oct 16 '18 at 19:43
  • @TimRandall i dont understand how entering a value of 'n' is a classic security vulnerability?can you please elaborate? – Tidem23 Oct 16 '18 at 19:46
  • If you search for "buffer overflow attack" there is plenty of information. The problem is that you're trusting the user to tell you how long a string is going to be. There's nothing to stop them entering more data than they promised. And where is their data going to go? Onto your stack, quite possibly. This was my initial guess as to what was wrong, but dbush got to the truth. – Tim Randall Oct 16 '18 at 19:50

1 Answers1

5

This is what happens when you mix scanf with gets / fgets.

The call to scanf is expecting a number. When you enter "1wjhue73b38409598" as the input it reads the 1 at the start of the string and stops reading when it sees the "w". So n gets set to 1.

Then gets immediately reads the remaining characters (i.e. "wjhue73b38409598") without prompting. Since the 1 is not part of this string, it doesn't get counted.

There's another problem however. Because n is set to 1, str can only hold 1 character. So the gets call writes past the end of the buffer. This invokes undefined behavior. In this particular case, things appeared to otherwise work, however a minor change to the code could change how undefined behavior manifests.

Even if you were to enter a proper value for n such as 50, the scanf call leaves a newline in the buffer. The subsequent call to gets then immediately reads the newline and stops, so all of the counters are 0.

Use a fixed size buffer instead and use fgets, which can be given a maximum size to read, to read the string:

char str[50];
fgets(str, sizeof(str), stdin);
dbush
  • 205,898
  • 23
  • 218
  • 273