3

i want to write a code to count occurrences of a specific hex number in a file. for example in the file there are 0x01 0x02 0x03 0x41 0x42 0x43 0x0D 0x0A 0xFF 0xFE 0xFD 0x01 0x02 0x03 0x80 0x7F 0x0D 0x0A and if i input : FF it will output: 1

i already working on the code but it doesn't seems to work

#include <stdio.h>
#include <stdlib.h>

main ()
{
    FILE *in, *out;
    unsigned char a[1000] = { 0 };
    int b;
    int count = 0, i = 0;
    in = fopen ("a.dat", "rb");
    out = fopen ("b.txt", "wb");
    while (!feof (in)) {
        b = fgetc (in);
        a[i] = b;
        i++;
    }

    scanf ("%x", &b);
    for (i = 0; i < 1000; i++) {
        if (a[i] == b) {
            count++;
        }
    }

    fprintf (out, "%d\n", count);
    printf ("%d\n", count);
    fclose (out);
    fclose (in);
    return 0;
}

(note: nesting error with '}' fixed)

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
cat
  • 33
  • 2
  • 3
    You will want to look at [Why is while ( !feof (file) ) always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) .. – David C. Rankin Jun 08 '22 at 04:54
  • Now would be a good time to learn to debug your own code. Run it in a debugger and trace the program as it runs. – kaylum Jun 08 '22 at 04:54
  • Note that using just `main() { … }` has been obsolete for the whole of the current millennium — you should use `int main(void) { … }` or possibly (but preferably not) `int main() { … }`. The return type is required by C99 and later versions of C, and you really should not be coding to C90 any more. Also, your unindented code is very hard to read — please indent using a standard indentation schema. – Jonathan Leffler Jun 08 '22 at 04:58
  • You are currently limited to files up to 1000 bytes long. You could easily handle arbitrarily large files by counting the frequency of each byte value in an array of 256 integers. If you read the value for the byte you're looking for, you don't even need the array — you can simply read each byte in turn and count just the ones you're interested in. You don't verify that the value read by `scanf("%x", &b);` is actually in the range 0x00..0xFF; you probably should. – Jonathan Leffler Jun 08 '22 at 05:06
  • Using `scanf()` without checking the return code is *asking* for trouble. You don't know if the user has actually entered a hex number (and `scanf()` succeeded) or not (making `scanf()` fail and leaving the erroneous input in the input buffer). – DevSolar Jun 08 '22 at 05:09
  • You open your file in binary mode. Yet you are asking about hex numbers. Hex numbers is a text representation of a certain value. Do you want to read text or binary data? Does your file contain 2-digit-hex-numbers (with prefix `0x` or without?) separated by a space or does it just contain binary data? – Gerhardh Jun 08 '22 at 05:37
  • @Gerhardh See the [previous iteration of this question](https://stackoverflow.com/questions/72529389), where it's clear that the file contains binary data. (Of course, it's always hard to talk about the values of the bytes in a binary file, without using words like "hex value", which then suggest that the file contains hex values.) – Steve Summit Jun 08 '22 at 13:45
  • @SteveSummit Thanks for the link. I knew it looked somehow familiar. ;) A good replacement for "hex value" might be "value" ;) – Gerhardh Jun 08 '22 at 13:52

1 Answers1

2

You have a large number of small errors. They can be summarized as follows:

  • while (!feof(in)) reads one-character-too-many. Look at the logic of the loop. After reading the last character, you check !feof(in) (which hasn't occurred yet) and then call b = fgetc (in); again (which now returns EOF), and then you blindly assign a[i] = b;. That's Why is while ( !feof (file) ) always wrong? Simply control your read-loop with the return from your read-function.
  • You use the wrong type with scanf(). %x requires an unsigned int* value, but you pass type int*. This will result in problems with signed and unsigned type mismatch. This is readily apparent when you compile with warnings enabled.
  • You fail to validate whether your open of in and out succeed. Always validate every file open operation.
  • You fail the validate the return of scanf(). You cannot use scanf() correctly unless you validate the number returned is equal to the number of valid conversions expected.
  • Since you write to b.txt you should validate the fclose(out). Always validate your close-after-write to ensure you catch any write error that occurs after the last value written by your code.
  • There is no need to loop over all 1000 elements of your array. You know the number of elements filled from the value of i. Just loop over the elements filled using a separate loop variable (j shown below).
  • Lastly, when you need input from your user, don't leave the user staring at a blinking cursor on the screen wondering whether the program is hung or what is going on, prompt the user for input.

Putting all the pieces together, you could do something similar to the following:

#include <stdio.h>
#include <stdlib.h>

#define MAXC 1000   /* if you need a constant, #define one (or more) */

int main (void)
{
    FILE *in, *out;
    unsigned char a[MAXC] = { 0 };
    int b;
    unsigned u;                   /* unsigned value required for scanf() */
    int count = 0, i = 0, j;
    
    in = fopen ("a.dat", "rb");
    out = fopen ("b.txt", "wb");
    
    if (!in) {  /* always validate every file open */
        perror ("fopen-a.dat");
        return 1;
    }
    
    if (!out) {  /* always validate every file open */
        perror ("fopen-b.txt");
        return 1;
    }
    
    /* protect array bound - use read function to control loop */
    while (i < MAXC && (b = fgetc (in)) != EOF) {
        a[i] = b;
        i++;
    }
    
    fputs ("enter 8-bit hex value to find: ", stdout);
    if (scanf ("%x", &u) != 1) {      /* validte every user-input */
        fputs ("error: invalid hex input.\n", stderr);
        return 1;
    }
    
    for (j = 0; j < i; j++) {     /* iterate over values read from file */
        if (a[j] == u) {
            count++;
        }
    }

    fprintf (out, "%d\n", count);
    printf ("%d\n", count);
    
    if (fclose (out) == EOF) {    /* always validate close-after-write */
        perror ("fclose-out");
    }
    fclose (in);
}

Example Use/Output

Compiling your code with full warnings enabled you can do:

$ gcc -Wall -Wextra -pedantic -Wshadow -std=c11 -O3 -o bin/readwriteucbin readwriteucbin.c

Running your code on the binary input provided you would get, e.g.

$ ./bin/readwriteucbin
enter 8-bit hex value to find: 0xff
1

Or where more than one value is matched, e.g.

$ ./bin/readwriteucbin
enter 8-bit hex value to find: 1
2
David C. Rankin
  • 81,885
  • 6
  • 58
  • 85