Reading .img file in C using open()

Question

I am trying to map a .img file, and I am not sure why my code is not working.

Here is my code, and when I run the code I keep getting my error, that p is equal to MAP_FAILED

int diskinfo(int argc, char* argv[]){

void *p;
char *size

    if (argc < 2) {
            printf("Please put ./diskinfo <file> \n");
            exit(1);
    }

    int fp = open(argv[1],"rb+");
    if(fp == NULL) {
            printf("Error opening file");
            exit(1);
    }

    struct stat buf;
    fstat(fp, &buf);

    p = mmap(NULL,buf.st_size, PROT_READ, MAP_PRIVATE, fp, 0);

    if(p == MAP_FAILED){
            printf("Error mapping memory\n");
            exit(1);
    }}

If anyone has any suggestions on where my code is wrong or if I am missing a piece of information I would be very grateful.

Changing to perror() does not work. Also changing this function doesn't change the fact that p is still equal to MAP_FAILED

if(p == MAP_FAILED){
        return;
}

I changed the below what is the solution:

int fp = open(argv[1],O_RDWR);
if(fp < 0){ 
    . . .

But I am still returning

Answer 1

It is unclear from the state of your question whether you were ever able to get mmap to work. Your last edit added:

int fp = open(argv[1],O_RDWR);

Which is fine if you are writing back to the file you have opened, but if not, you should open using O_RDONLY to prevent inadvertent modification of your original file.

While not an error, fp is generally used as a file pointer associated with file stream operations when the file is opened with fopen. Here you are using low-level I/O with read/write which uses a file descriptor instead of a stream pointer. When referencing a descriptor, the general vernacular uses fd as short-hand for file descriptor. (personally, it was awkward to see the two used in a interchanged manner -- which I suspect is the case for others as well)

Your remaining use of fstat, the resulting buf.st_size and your call to mmap are not the problem. Your problem lies elsewhere -- which is one of the primary reasons you should post A Minimal, Complete, and Verifiable Example (MCVE).

That said, to insure you have incorporated your changes in the proper manner, I'll leave you with a simple example that mmaps a file and simply dumps the file to stdout (so limit your input filenames to a reasonably short text file to work with the example -- otherwise you will see all sorts of strange characters). Work through the following:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

int diskinfo (int argc, char *argv[]) {

    char *p = NULL;         /* pointer to mmapped file */
    int fd = 0;             /* file descriptor */
    struct stat buf = {0};  /* struct stat */
    ssize_t size = 0;       /* file size (typed for write return) */

    if (argc < 2) { /* validate at least 2 arguments */
        printf ("Please put %s <file> \n", argv[0]);
        exit (EXIT_FAILURE);
    }

    if ((fd = open (argv[1], O_RDONLY)) == -1) { /* open/validate file */
        perror ("Error opening file");
        exit (EXIT_FAILURE);
    }

    if (fstat (fd, &buf) == -1) {       /* stat file for size */
        perror ("error: fstat buf");
        exit (EXIT_FAILURE);
    }
    size = buf.st_size;                 /* get file size */

    /* mmap file and validate return */
    if ((p = mmap (NULL, buf.st_size, PROT_READ, MAP_PRIVATE, fd, 0)) 
                    == (void *) -1) {
        perror ("mmap failed");
        exit (EXIT_FAILURE);
    }

    /* simple example, output mmapped file to stdout */
    if (write (STDOUT_FILENO, p, size) != size) {
        perror ("error on write");
        exit (EXIT_FAILURE);
    }

    munmap (p, size);   /* unmap file */

    return 1;   /* return success (fn could be void due to exit) */
}

int main (int argc, char **argv) {

    diskinfo (argc, argv);  /* call diskinfo function */

    return 0;
}

(note: your check of if (argc < 2) should really be done in the calling function, main() here. There is no reason to call diskinfo until you have validated you have a filename to open. You could actually refactor your code to check the arguments and open the file in main() and simply pass an open file-descriptor to diskinfo as a parameter)

Example Use/Output

$ ./bin/mmapdiskinfo dat/captnjack.txt
This is a tale
Of Captain Jack Sparrow
A Pirate So Brave
On the Seven Seas.

Look things over and let me know if you have any questions. If you still cannot get your function to work, then post a MCVE so that we can help further.

Answer 2

Your code is not working at the beginning, check your open call.

If you compiling with proper flags like -Wall -Werror you should get warnings like this:

error: passing argument 2 of ‘open’ makes integer from pointer without a cast [-Werror]
note: expected ‘int’ but argument is of type ‘char *’

You should designate open and fopen functions, they are different things.

int open(const char *pathname, int flags);