Peculiar memory/printing issue C

Question

#include<stdio.h>
#include<stdlib.h>
#include <string.h>
int main(void){
char dnachar [10]
int cytosine; //first int
int thymine;  //second int
int guanine;
int adenine;
printf("Enter DNA Sequence ");
scanf("%10s",dnachar);
printf("dna:%s\n", dnachar);

 for (int i = 0; i < 10; i++){

     char x = dnachar[i];
     if(x == 'c')
     cytosine++;
     if(x == 't')
     thymine++;
     if(dnachar[i] == 'g')
     guanine++;
     if(dnachar[i] == 'a')
     adenine++;
 }
printf("%d,%d,%d,%d",thymine, guanine, cytosine, adenine);
return 0;
}

Hi, I am new to C and I noticed that the program only prints the first two ints accurately - cytosine and thymine. The others show a random string of incoherent numbers. Why is that and how can I fix it?

edit: I changed i<20 to i<10, I don't know how I made that mistake.

Your code invokes *undefined behavior*. `char x = dnachar[i];` in a loop from 0..19 inclusive, how do you expect that to work when `char dnachar[10]` is the formal declaration? Your loop breaches memory access beyond your declared array size. Further, *none* of your declared `int` variables are determinate. They have no initial value formally defined. More undefined behavior. Your compiler should be spewing warnings about the latter issue, and if it isn't, turn up your warning levels and above all, treat them as errors because that's exactly what they are. — WhozCraig, Jun 20 '20 at 10:20
Hey WhozCraig, I always had my for loop at i<10, I don't know how I posted it as i<20. Good observation! I am not getting any errors though, I am using the 'Wall' and the cs50 sandbox if that helps . — user209835, Jun 20 '20 at 10:33
@user209835 Sometimes they are, yes. But not in this case. See https://stackoverflow.com/q/1597405/1679849 — r3mainer, Jun 20 '20 at 10:44

score 0 · Accepted Answer · answered Jun 20 '20 at 10:42

Among the multitude of things wrong in this code

Wrong format limiter

Given:

char dnachar[10];

The code:

scanf("%10s",dnachar);

will read a string up to 10 characters long, not including the terminating null, which it will always place. Therefore, that scanf can write as many as eleven (11) chars in a space only declared to hold ten (10). The result of a ten character string read (or longer) will be undefined behavior.

Solution:

if (scanf("%9s", dnachar) != 1)
    return EXIT_FAILURE;

Note: if your sequences are really ten characters each, then dnachar should be declared as:

char dnachar[11]; // account for 10-chars + terminator

And the format string for scanf should be %10s accordingly.

Indeterminate Counters

All four of your counters are indeterminate. They have no specified initial value as automatic variables, and therefore operations expecting predictable results are unfounded. If they had static linkage (either globals or declared static) they would have default values of 0, but that isn't what your code does.

Solution:

int cytosine = 0;
int thymine = 0;
int guanine = 0;
int adenine = 0;

Incorrect Loop Limit

Given:

 for (int i = 0; i < 20; i++){
     char x = dnachar[i];

This assumes there are twenty characters in dnachar (which is impossible unless you've invoked undefined behavior from the item above). One you access dnachar[10] and beyond, more undefined behavior. Ideally you stop processing chars when there are no more, which is much easier with a pointer than an index.

Solution:

for (char *p = dnachar; *p; ++p)
{
    switch(*p)
    {
        case 'c':
            ++cytosine;
            break;
    
        case 't':
            ++thymine;
            break;
    
        case 'g':
            ++guanine;
            break;
    
        case 'a':
            ++adenine;
            break;
    
        default:
            break;
    }
}

Peculiar memory/printing issue C

1 Answers1