-1

When I tried to write codes that print the codes in my file, I had a error message that "Exception has occurred. EXC_BAD_ACCESS (code=1, address=0x68)".

I have googled it, but no sulution have I found.

I need someone's help.

Thank you

I thought that this was because of memory allocation, so I tried to use malloc and ordinary array. However, neither of them didn't work.

This is the code.

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

int main()
{
    FILE* fp;
    fp = fopen("pO5.c", "rb");
    // char buf[100];
    char* buf;
    while (1) {
        buf = (char*)malloc(sizeof(buf) * 1000);
        int n = fread(buf, 1, 100, fp);
        if (n == 0) {
            break;
        }
        fwrite(buf, 1, n, stdout);
    }
    fclose(fp);
    return 0;
}

This code is expected to print the code itself.

xksa
  • 87
  • 1
  • 8
  • 1
    You need to check the return value of `fopen`. `fopen` can fail. – melpomene Jul 28 '19 at 07:26
  • Thanks! But even if I fix it, the program doesn't print the contents of this file.. – xksa Jul 28 '19 at 07:32
  • If you use `malloc`, use `free` as well. And check the return value of malloc, it could be NULL. What you have here is a program allocating in a loop, as long as the file has data. Think about what will happen when the file is very large. Also: why allocate 1000 but only read 100? And why iterate once more if fread returns not 0, but also not 100? Anyway: if you learn how to use a debugger, you'll be able to get an answer for much of these problems yourself. – stijn Jul 28 '19 at 07:33
  • Oh, I got it! I changed my code as you said and changed the filename to "./p05.c". Really thank you! – xksa Jul 28 '19 at 07:34
  • `./` is redundant, but it looks like you also changed `O` to `0`. – melpomene Jul 28 '19 at 07:35
  • `“EXC_BAD_ACCESS”` can mean several things, but commonly it means to you abusing a pointer -- badly. Your error speaks volumes `address=0x68`, an address of `0x68` is at the very bottom of the *System Reserved* memory range -- you can't read or write there... thus, the Bad Access. – David C. Rankin Jul 28 '19 at 08:34

1 Answers1

4

Here are few observation. Firstly, here

fp = fopen("pO5.c", "rb");

one should always check the return type of fopen() to know whether call to fopen() was success of failure. It may cause issue if fopen() failed & you are operating on fp further. Proper error handling is required, for e.g

fp = fopen("pO5.c", "rb");
if(fp == NULL) {
   fprintf(stderr, "Can't open %s: %s\n", "pO5.c", strerror(errno)); /* include header for errno */
   exit(EXIT_FAILURE);
}

Secondly, here

buf = (char*)malloc(sizeof(buf) * 1000);
int n = fread(buf, 1, 100, fp);

don't use magic number like 100 or 1000, instead its advisable to use macro for this purpose.

Also sizeof(buf) is size of pointer, but you wanted it to be sizeof(*buf). And typecasting malloc() result is not required here. For e.g

buf = malloc(sizeof(*buf) * BUF_MAX_SIZE); /* define BUF_MAX_SIZE */

Also do check return value of malloc() for e.g

buf = malloc(sizeof(*buf) * BUF_MAX_SIZE); /* define BUF_MAX_SIZE */
if(buf == NULL) {
  /* @TODO error handling if malloc failed */
}

And below code block is flawed

while (1) {
      buf = (char*)malloc(sizeof(buf) * 1000); 
      int n = fread(buf, 1, 100, fp);
      if (n == 0) {
          break;
      }
      fwrite(buf, 1, n, stdout);
}

Because of mainly one reason

  • you are not freeing each malloc'ed memory ? Its memory leak as every time buf reassigned with new dynamic address & no object or pointer pointing to previously allocated memory.

One solution for the above issue can be this one

buf = malloc(sizeof(*buf) * BUF_MAX_SIZE); /* calling malloc() n times costs more, it can be avoided by allocating memory before while(1) block */
if(buf == NULL) {
   /* @TODO error handling of malloc */
}

int n = 0; /* declare here */
while (1) {     
   n = fread(buf, 1, BUF_MAX_SIZE, fp); /* reading BUF_MAX_SIZE bytes every time from file & storing into buf */
   if (n == 0) {
       break;
   }
   fwrite(buf, 1, n, stdout);/* put buf to stdout */ 
   /* zero'd the BUF_MAX_SIZE bytes of buf, can use memset() here */
 }       
 free(buf); /* free here */
Achal
  • 11,821
  • 2
  • 15
  • 37
  • Thanks @DavidC.Rankin Yours comments means a lot. – Achal Jul 28 '19 at 08:41
  • `fopen` error messages should include the filename and the reason for the failure: `fprintf(stderr, "Can't open %s: %s\n", "pO5.c", strerror(errno));` Also, `exit(EXIT_FAILURE);` to indicate failure to your caller. – melpomene Jul 28 '19 at 09:30
  • Agree @melpomene that's really better one. – Achal Jul 28 '19 at 09:32
  • The `break` when n is 0 will leak the memory from the last iteration. Just allocate the memory once, outside of the loop? – stijn Jul 28 '19 at 10:48