Buffer filling last empty spaces in char array with unwanted data

Question

FILE* file_;
        char buffer[5];
                file_ = fopen("Data.txt", "a+");
        while (!feof(file_)) 
        {
            fread(buffer, sizeof(buffer), 1, file_);
            cout << buffer<<" ";
            BM(buffer, pat);
        }

Data.txt="ABCC1ABCC2XXX"

Output:

ABCC1m

ABCC2m

XXXC2m

How can I make buffer stop before it starts generating chars from previous buffer?(bolded font part)

Wanted output:

ABCC1

ABCC2

XXX

Are you randomizing your indentation? Anyway, the problem is you're missing the C string's null terminator. I'll see if I can find a good duplicate ... but since you tagged C++, you could actually use std::string and std::ifstream and save yourself a lot of effort. — Useless
– Useless, Commented May 28, 2019 at 9:34
Change sizeof(buffer) to sizeof(buffer) - 1, in order to avoid overriding the null character. — goodvibration
– goodvibration, Commented May 28, 2019 at 9:36
@goodvibration You need to put a null character in there. There isn't one by default. — Martin Bonner supports Monica
– Martin Bonner supports Monica, Commented May 28, 2019 at 9:37
@MartinBonner: I haven't worked with fread in a while, so I've figured out of the back of my memory that it adds the null character after the amount of characters requested. — goodvibration
– goodvibration, Commented May 28, 2019 at 9:39

Martin Bonner supports Monica · Accepted Answer · 2019-05-28 09:53:18Z

5

You have two problems. Firstly you are not null-terminating buffer at all, (which is why you are getting the m output. Secondly, you are not null-terminating buffer in the right place when there is a short read.

fread will tell you how many characters it has read, and you need to put the '\0' there. Edit That previous description is not accurate. It tells you how many objects it has read, each of size arg2, and you read upto arg3 of them. You need to change the arguments to fread so that you are reading single characters, and as many of them as there room in the buffer. So:

        FILE* file_;
        char buffer[5+1];
                file_ = fopen("Data.txt", "a+");
        while (!feof(file_)) 
        {
            const size_t nchars = fread(buffer, 1, sizeof(buffer)-1, file_);
            buffer[nchars] = '\0';
            cout << buffer<<" ";
            BM(buffer, pat);
        }

Pedantic note: The second argument to fread could also be written as sizeof(buffer[0]) if buffer is of something other than char/signed char/unsigned char - but those three are defined to have a sizeof 1.

edited May 28, 2019 at 9:53

answered May 28, 2019 at 9:36

Martin Bonner supports Monica

29.1k3 gold badges56 silver badges91 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Steve Over a year ago

nchars should be size_t, shouldn't it?

goodvibration Over a year ago

What's i by the way?

Martin Bonner supports Monica Over a year ago

@Steve Good point - and fread doesn't return -1 on error (so I've removed that).

Martin Bonner supports Monica Over a year ago

@goodvibration The variable now known as nchars. I initially called it i (local variable, only valid for a small scope, don't need to think of a good name), then realized nchars would be a better name - but didn't change the uses!

goodvibration Over a year ago

You also checked if it was smaller than 0, which doesn't make much sense either if you intended for it to be the value returned from fread as you've just explained (since this function never returns a value smaller than 0).

|

Useless · Accepted Answer · 2019-05-28 09:59:55Z

OK, I can't find a decent duplicate - maybe someone else will.

The line

fread(buffer, sizeof(buffer), 1, file_);

potentially fills your buffer completely. You need to keep the return value to know how many bytes were actually written, but assuming your file contained at least five bytes, all five bytes of your buffer array are now initialized.

However, to print buffer as a regular C string, it needs a sixth byte, containing the null terminator.

For example, the C string "12345" is actually represented as the char array {'1', '2', '3', '4', '5', 0}.

You don't have room for a terminator in your buffer, and don't write one, so you can't treat it as a simple C string.

Your options are:

add a terminator manually, as in Martin Bonner's answer
don't add a terminator, but track the size - you can use the C++17
```
std::string_view bufstr(buffer, nchars);
```
to keep the pointer and size together (and you can print this normally)
stop using the old C I/O library entirely. The C++ I/O library admittedly doesn't have a much better way to read groups of five characters, but reading whole lines, for example, is much easier to do correctly.

Thanks! Giving the OP the proximate solution quickly is always worthwhile too, though, and it's often more digestible.

Collectives™ on Stack Overflow

Buffer filling last empty spaces in char array with unwanted data

2 Answers 2

6 Comments

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

2 Comments

Related