0

I can't seem to work out how to convert from an fopen to open. I don't have much c experience, so this is quite overwhelming for me.

Here is what it is on its own:

In the cache_reader.c file (just the open and close functions):

void cr_close(cr_file* f){
free(f->buffer);
fclose(f->file);
}

cr_file* cr_open(char * filename, int buffersize)
{
    FILE* f;
    if ((f = fopen(filename, "r")) == NULL){
         fprintf(stderr, "Cannot open %s\n", filename);
         return 0; }

    cr_file* a=(cr_file*)malloc(sizeof(cr_file));
    a->file=f;
    a->bufferlength=buffersize;
    a->usedbuffer=buffersize;
    a->buffer=(char*)malloc(sizeof(char)*buffersize);
    refill(a);
    return a;
 }

In the cache_reader.h file:

typedef struct{
   FILE* file;        //File being read
   int bufferlength;  //Fixed buffer length
   int usedbuffer;    //Current point in the buffer
   char* buffer;      //A pointer to a piece of memory
                 //  same length as "bufferlength"
 } cr_file;
 //Open a file with a given size of buffer to cache with
 cr_file* cr_open(char* filename, int buffersize);
 //Close an open file
 void cr_close(cr_file* f);
 int refill(cr_file* buff);

In the cache_example.c file:

int main(){
char c;

 //Open a file
 cr_file* f = cr_open("text",20);  

 //While there are useful bytes coming from it
 while((c=cr_read_byte(f))!=EOF)
 //Print them
 printf("%c",c);

 //Then close the file
 cr_close(f);

//And finish
return 0;
}

I know that I need to change fclose to close, fopen to open. But I do not understand most of the other stuff. I am getting a ton of errors for everything, and I am not sure how this works out with pointers, as I barely have any experience with them. I tried to use int fileno(FILE *stream), by saying int fd = fileno(FILE *f) and then fd = fopen(filename, "r")) == NULL). This didn't work, and all examples of the open function I could find just use the name of the file, rather than a pointer of chars to specify filename... I thought the cr_close function can be done by just changing fclose to close, but this doesn't work either. I am not sure if I need to edit cache_example.c file as well.

Could anyone provide some help to put me on the right path to do this..?

Papbad
  • 143
  • 1
  • 11
  • 4
    What do you think is the difference between the name of the file and a pointer to `char`? – Jonathan Leffler Feb 27 '18 at 14:45
  • 2
    The object of the exercise is to leave the example code unchanged, but reimplement the other code to use file descriptors instead of file streams. Sadly, the header exposes the internals of the structure, unnecessarily, so the example needs to be recompiled. You will change the `FILE *` member to an `int`. You will not use any function that takes a file stream argument. – Jonathan Leffler Feb 27 '18 at 14:52
  • How do I change FILE * to int..? I thought that the fileno function would do that which I tried to do. I also tried to change fprintf to printf and stderr to stdout to make it work. It still didn't. – Papbad Feb 27 '18 at 14:59
  • Note: change `char c` --> `int c` to avoid incorrect behavior. – chux - Reinstate Monica Feb 27 '18 at 15:00
  • Related: https://stackoverflow.com/q/1658476/1531971 –  Feb 27 '18 at 15:00
  • To change from `FILE *` to `int`, you use an editor. In `vim`, you edit `cache_reader.h` and use `/FILE`, return, and `:s/FILE./int/` and return — one amongst many possibilities, but easier to put in comments than some of the alternatives. – Jonathan Leffler Feb 27 '18 at 15:18
  • @JonathanLeffler I don't really understand... my file has a struct, can't I not store the file descriptor in it to then use it...? What do you mean when you say return..? What code exactly would this be and where would I place it inside the cache_reader.h file..? Do you mean return as in Im suppose to store it in some variable..? Im really bad at C I started maybe a month ago, and my other programming experience is not much either.. – Papbad Feb 27 '18 at 15:28
  • The header (`cr_file.h`) should contain: `typedef struct cr_file cr_file;` (and not the structure definition). The source (`cr_file.c`) should contain: `struct cr_file { int file; int bufferlength; int usedbuffer; char* buffer; };` This gives you an opaque type in the client (example) code, and allows you to change the structure without needing to recompile the client code (though you would have to recompile the implementation, of course — we can't work complete miracles). – Jonathan Leffler Feb 27 '18 at 15:34
  • You should rename `refill()` to `cr_refill()`, or (more likely) make it `static` because user-code would not call it directly (or both). – Jonathan Leffler Feb 27 '18 at 15:42
  • @JonathanLeffler ..but why can't I recompile the code..? I compile it using a 'make' file provided with the code. I don't understand how this is needed to use the open function. Why is this required to convert to an open function..? – Papbad Feb 27 '18 at 15:43
  • Of course you _can_ recompile the code. However, it is often more convenient in the long run if you can make changes and improvements to your library code without requiring the consumers (other programmers) to recompile their code. Binary compatibility is very important for big libraries, like the standard C library on a given platform. For a tiny project like this, it doesn't matter — but you'll need to learn about the larger scale issue too, at least in due course, if you stick with programming as a career. – Jonathan Leffler Feb 27 '18 at 15:58

2 Answers2

1

From comments

The object of the exercise is to leave the example code unchanged, but reimplement the other code to use file descriptors instead of file streams. Sadly, the header exposes the internals of the structure, unnecessarily, so the example needs to be recompiled. You will change the FILE * member to an int. You will not use any function that takes a file stream argument.

The header (cache_reader.h) should contain this (and not the structure definition):

typedef struct cr_file cr_file;

The source (cache_reader.c) should contain:

struct cr_file
{
    int file;
    int bufferlength;
    int usedbuffer;
    char *buffer;
};

This gives you an opaque type in the client (example) code, and allows you to change the structure without needing to recompile the client code (though you would have to recompile the implementation, of course — we can't work complete miracles).

Of course you can make your clients recompile their code whenever you make changes to the internals of the library. However, it is often more convenient in the long run if you can make changes and improvements to your library code without requiring the consumers (other programmers) to recompile their code. Binary compatibility is very important for big libraries, like the standard C library on a given platform. For a tiny project like this, it doesn't matter — but you'll need to learn about the larger scale issue too, at least in due course, if you stick with programming as a career.

Reworked code

Note that I concluded I needed some different members to support your use case — I needed to know the size of the allocated buffer, the amount of data actually in the buffer, and the current position for reading. I renamed the members to bufmax (your bufferlength), bufpos (your usedbuffer), and added buflen.

I wrote sample code for cr_read_byte(), and it works reading a file.

However, there is considerable work to be done to support writing too, and moving around in the file other than one byte at a time, and so on.

cache_reader.h

#ifndef CACHE_READER_H_INCLUDED
#define CACHE_READER_H_INCLUDED

typedef struct cr_file cr_file;

extern cr_file *cr_open(char *filename, int buffersize);
extern void cr_close(cr_file *f);
extern int cr_read_byte(cr_file *f);

#endif /* CACHE_READER_H_INCLUDED */

cache_reader.c

#include "cache_reader.h"
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

struct cr_file
{
    int   file;     // File being read
    int   bufmax;   // Fixed buffer length
    int   bufpos;   // Current point in the buffer
    int   buflen;   // Amount of data in the buffer
    char *buffer;   // A pointer to a piece of memory
};

static void cr_refill(cr_file *f)
{
    if (f->bufpos >= f->buflen)
    {
        int nbytes = read(f->file, f->buffer, f->bufmax);
        if (nbytes > 0)
        {
            f->buflen = nbytes;
            f->bufpos = 0;
        }
    }
}

void cr_close(cr_file *f)
{
    free(f->buffer);
    close(f->file);
    free(f);
}

cr_file *cr_open(char *filename, int buffersize)
{
    int fd;
    if ((fd = open(filename, O_RDWR)) < 0)
    {
        fprintf(stderr, "cannot open %s for reading and writing (%d: %s)\n",
                filename, errno, strerror(errno));
        return 0;
    }

    cr_file *a = (cr_file *)malloc(sizeof(cr_file));
    char *b = (char *)malloc(sizeof(char) * buffersize);
    if (a == 0 || b == 0)
    {
        free(a);
        free(b);
        close(fd);
        fprintf(stderr, "cannot allocate %zu bytes of memory (%d: %s)\n",
                sizeof(cr_file) + buffersize, errno, strerror(errno));
        return 0;
    }
    a->file = fd;
    a->bufmax = buffersize;
    a->bufpos = 0;
    a->buflen = 0;
    a->buffer = b;
    return a;
}

int cr_read_byte(cr_file *f)
{
    if (f->bufpos >= f->buflen)
        cr_refill(f);
    if (f->bufpos >= f->buflen)
        return EOF;
    return f->buffer[f->bufpos++];
}

cache_example.c

#include "cache_reader.h"
#include <stdio.h>

int main(void)
{
    cr_file *f = cr_open("text", 20);
    if (f != 0)
    {
        int c;
        while ((c = cr_read_byte(f)) != EOF)
            putchar(c);
        cr_close(f);
    }
    return 0;
}

makefile

CFLAGS  = -std=c11 -O3 -g -Wall -Wextra
LDFLAGS =
LDLIBS  =

FILES.c = cache_example.c cache_reader.c
FILES.o = ${FILES.c:.c=.o}
FILES.h = cache_reader.h

PROG1 = cache_example

PROGRAMS = ${PROG1}

all: ${PROGRAMS}

${PROG1}: ${FILES.o}
    ${CC} -o $@ ${CFLAGS} ${FILES.o} ${LDFLAGS} ${LDLIBS}

${FILES.o}: ${FILES.h}

You can find this code (except for the skeletal makefile shown in the answer) on GitHub in my SOQ (Stack Overflow Questions) repository as files in the src/so-4901-1302 sub-directory.

Jonathan Leffler
  • 666,971
  • 126
  • 813
  • 1,185
  • That's about as complete a lesson as you can give professor Leffler. – David C. Rankin Feb 28 '18 at 05:06
  • @DavidC.Rankin: Thanks. I felt that the other answer wasn't really helping all that much, though there's some danger this is going too far. I haven't pointed out all the subtleties in the code, but that would probably be in danger of overwhelming the OP. – Jonathan Leffler Feb 28 '18 at 05:16
  • I always appreciate the well laid out and reasoned examples, I learn something every time -- and that's a good day. – David C. Rankin Feb 28 '18 at 05:35
  • I doubt I can fully appreciate the little things because of my current level of programming, but I have been able to convert my code accordingly by looking at this one to get what I wanted. Thank you for your time. – Papbad Feb 28 '18 at 15:43
0

the following proposed code:

  1. cleanly compiles
  2. eliminates the unneeded/unused code
  3. includes the #include statements and why they are needed
  4. performs the desired functionality
  5. uses (per the OPs request) file descriptors rather than file pointers

and now, the proposed code

// following struct not actually used in this code
#if 0
typedef struct
{
   int fd;               // file descriptor number of input file
   /*
    * int bufferlength;  // Fixed buffer length
    * int usedbuffer;    // Current point in the buffer
    * char* buffer;      // A pointer to a piece of memory
    *                    // same length as "bufferlength"
    * */
} cr_file;
#endif
-------------------------

// following 3 header files for 'open()' and 'O_RDONLY'
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

// following for 'read()' and 'close()'
#include <unistd.h>

// following for 'exit()' and 'EXIT_FAILURE'
#include <stdlib.h>

// following for 'printf()', 'perror()'
#include <stdio.h>

int main( void )
{
    //Open a file
    int fd = open("text", O_RDONLY);
    if( 0 > fd )
    { // then open failed
        perror( "open for input file failed" );
        exit( EXIT_FAILURE );
    }

    // implied else, open successful

    //While there are useful bytes coming from it
    char buf;

    while( read( fd, &buf, 1 ) > 0 ) 
    {
        printf( "%c", buf );
    }

    //Then close the file
    close( fd );

    //And finish
    return 0;
}
user3629249
  • 15,593
  • 1
  • 16
  • 17