3

When I write simple code to encode simple sequence of letters to bytes and decode again I have problem with decoding. Everything is going on by in finish I want to have sequence of 4 chars, but it includes also bytes on the end. Here is my code:

char* B2T(int num) {
    unsigned char temp;
    char res[4];
    int sw[] = { 6,4,2,0 };
    char tab[4] = { 'A', 'C', 'G', 'T' };
    int i = 0;
    for (int i = 0; i < 4; i++) {
        res[i] = tab[(num >> sw[i]) & 3];
    }
    printf_s("%s\n", res); //!!!!!!problem here!!!!!!!!
    return res;
}

int main() {
    FILE *I, *O;
    char tab[5], opt;
    int res, i, temp;
    bool work = true;
    while (work) {
        printf_s("\nChoose option: decode or encode (d/e): ");
        scanf_s("%c", &opt);
        switch (opt) {
        case 'e':
            fopen_s(&I, "DNA.txt", "r");
            fscanf_s(I, "%s", &tab, 5);
            fopen_s(&O, "result.bin", "a");
            while (feof(I) == 0) {
                res = T2B(tab);
                printf_s("%X ", res);
                fprintf_s(O, "%X ", res);
                fscanf_s(I, "%s", &tab, 5);
            };
            fclose(I);
            fclose(O);
            break;
        case 'd':
            fopen_s(&I, "result.bin", "r");
            fscanf_s(I, "%X", &temp);
            while (feof(I)==0) {
                char* ress = B2T(temp);
                fscanf_s(I, "%X", &temp);
            }
            fclose(I);
            break;
        }
    }
    return 0;
}
Ron
  • 14,674
  • 4
  • 34
  • 47
daniktl
  • 63
  • 3

3 Answers3

3

You populate char res[4];, without null-terminating it, which causes Undefined Behavior, since printf() expects the null-terminating symbol to stop printing.

Do that instead:

char res[5];
res[4] = '\0';

Moreover, you should focus on this line:

while (feof(I) == 0)

which uses feof() inside a loop to stop parsing the file. This is a known issue, which explains your extra character. Please read Why is “while ( !feof (file) )” always wrong?


PS: In general all functions of the C library expect a string to be null-terminated, so it's strongly advised to have all your strings null-terminated.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
2

Oups! Others already said that the wrong output was caused by a char array not null terminated.

But your code contains another (more serious IMHO) flaw: you are returning an automatic array from a function:

char* B2T(int num) {
    unsigned char temp;
    char res[4];              // <- res will reach end of life when function returns
    ...
    return res;
}

int main() {
    FILE *I, *O;
    char tab[5], opt;
    int res, i, temp;
    ...          
                res = T2B(tab);  // <- res is a dangling pointer converted to int
    ...

Using a pointer to a an array after it reaches end of life is explicitely Undefined Behaviour. In common implementations, the automatic arrays is stored in the stack, and its memory can be reused after the function has returned leading to unexpected changes. Google for C dangling pointer for more references...

A quick fix is to declare it static, which can be acceptable here because you use neither recursion nor multi-threading:

char* B2T(int num) {
    unsigned char temp;
    static char res[5];              // <- res will persist after function returns
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
1

Your res is not null-terminated.

change it to:

char res[5];
res[4] = '\0';

Then the printf will print it correctly.

SHR
  • 7,940
  • 9
  • 38
  • 57