0

I wrote this little program that reads a file in binary form (Databases.db in this example) and copies its content in the cpydatabases.db...

When I run the debugger in the fopen_s(&source, "Databases.db", "r");, the source is always NULL (while debugging it shows that the memory entry is always Null, 0x000000000000 <NULL>).

This program runs in visual studio 2015.

#include <stdio.h>
#include <stdlib.h>
#include <stdio.h>
#include "dirent.h"
#include <string.h>
#include <sys/stat.h>
#include <stdlib.h>

#define BUFFSIZE 2048
char ch, *readbuf;
int nread, nwrit;
FILE *source, *target;

int main()
{
    int returnv;
    fopen_s(&source, "Databases.db", "r");

    if ( source !== NULL)
    {
        fclose(source);
        return (EXIT_FAILURE);
    }
    fopen_s(&target,"cpydatabases.db", "w");
    //check again
    if (target == NULL)
    {
        fclose(target);
        return(EXIT_FAILURE);
    }
    //setting the char that reads the binary
    readbuf = (char *)malloc(BUFFSIZE* sizeof(char));

    if (readbuf == NULL)
    {
        fclose(source);
        fclose(target);
        return(EXIT_FAILURE);
    }

    while (1)
    {
        nread = fread((void *)readbuf, sizeof(char), BUFFSIZE, source) ;
        // fwrite((void *)readbuf, sizeof(char), nread, target);
        nwrit = fwrite((void *)readbuf, sizeof(char), nread, target);
        if (nwrit < nread)
        {
            returnv = (EXIT_FAILURE);
        }
        if (nread <= BUFFSIZE)
        { 
            returnv = (EXIT_SUCCESS);
            break;
        }    
    }

    fclose(source);
    fclose(target);

    return 0;
}
egoist
  • 1
  • 3
  • What is the return of `fopen_s` function calls? If `source` is `NULL`, you must not `close()` it. – Mathieu Dec 13 '16 at 09:24
  • Check the msdn example for the return value of fopen_s https://msdn.microsoft.com/en-us/library/z5hh6ee9.aspx – izlin Dec 13 '16 at 09:26
  • 1
    Are the files located in the same folder as the executable? – Bauss Dec 13 '16 at 09:28
  • Note: the memory you've `malloc`ated is not `free`d. – Mathieu Dec 13 '16 at 09:30
  • Why can't you use `fopen` and, when it fails, use `perror`? Like [here](http://stackoverflow.com/a/18193383/841108)? – Basile Starynkevitch Dec 13 '16 at 09:30
  • i used the perror in my code returning erron n16 which is my exact problem... – egoist Dec 13 '16 at 10:39
  • i think https://msdn.microsoft.com/en-us/library/ktss1a9b.aspx link might be useful. – CodeWarrior101 Dec 13 '16 at 11:09
  • Files that are opened by fopen_s and _wfopen_s are not sharable. If you require that a file be sharable, use _fsopen, _wfsopen with the appropriate sharing mode constant—for example, _SH_DENYNO for read/write sharing. – CodeWarrior101 Dec 13 '16 at 11:25
  • this didn't work either...the dublicated file is still null – egoist Dec 13 '16 at 22:20
  • this code block: ` if ( source !== NULL) { fclose(source); return (EXIT_FAILURE); }` is not correct: it will exit immediately when ever the input file was successfully opened. Suggest: ` if ( source == NULL) { return (EXIT_FAILURE); }` so only exits if `fopen()` fails – user3629249 Dec 14 '16 at 14:43
  • the code block: ` if (target == NULL) { fclose(target); return(EXIT_FAILURE); }` is not correct, The is NOT open, so no need to close it. However, the source is open, so it needs to be closed. Suggest: ` if (target == NULL) { fclose(source); return(EXIT_FAILURE); }` – user3629249 Dec 14 '16 at 14:46
  • When a system function, such as: `fopen()` fails, an error message should be output to stderr that contains why the system/OS thinks the call failed. The easiest way to do that is to insert the statement: `perror( "fopen failed for source" ); before any other statements in the error handling code – user3629249 Dec 14 '16 at 14:48
  • in C, the heap allocation functions (malloc, calloc, realloc) return type is `void*` which can be assigned to any other pointer. Casting the returned value just clutters the code, making it more difficult to understand, debug, maintain. The expression: `sizeof(char)` is defined in the standard as 1. Multiplying anything by 1 has no effect. Using that expression in the parameter to `malloc()` just clutters the code. Suggest removing that expression – user3629249 Dec 14 '16 at 14:52
  • in general, it is a bad programming paradigm to use global variables when those variables will not be used in another file. Suggest moving them to local variables, on the stack inside the `main()` function – user3629249 Dec 14 '16 at 14:57
  • the posted code sets the variable `returnv` in a number of places, but never uses that value. When compiling, always enable all the warnings, then fix those warnings – user3629249 Dec 14 '16 at 14:59
  • when calling `fread()` (and usually when calling `fwrite()`) always check the returned value to note how many of the second parameter size 'objects' were actually read. – user3629249 Dec 14 '16 at 15:03
  • the returned values from `fread()` and `fwrite()` should be check to be >= 0. and if <0 then handle the error – user3629249 Dec 14 '16 at 15:04
  • this code block: `if (nread <= BUFFSIZE) { returnv = (EXIT_SUCCESS); break; }` say that if the read were successful (or an error occurred) then exit the read/write loop. Not what the code really wants to do. A read count of 0 indicates EOF and then you would want to exit the read/write loop. – user3629249 Dec 14 '16 at 15:07
  • the function: `fopen_s()` is MSN specific (I.E. Not portable) Suggest adding `#define _crt_secure_no_warnings` at the top of the file and then using `fopen()` rather than `fopen_s()` so your code will be portable. – user3629249 Dec 14 '16 at 15:17
  • thank you very much man you've been like extra helpfull!!! i'll arrange my project to your suggestions! – egoist Dec 14 '16 at 22:30

2 Answers2

1

This worked for me. You should have your Databases.db file in the same folder as your source.cpp file, or use an absolute path like "C:/Databases". Anyway this code worked for me:

#define BUFFSIZE 2048
char ch, source_file[50], target_file[50], *readbuf;
int nread, nwrit;

FILE *source, *target;
int main()
{

    int returnv;
    fopen_s(&source, "Databases.db", "r");

    if (source == NULL)
    {
        //fclose(source);
        return (EXIT_FAILURE);
    }
    fopen_s(&target, "cpydatabases.db", "w");
    //check again
    if (target == NULL)
    {
        fclose(target);
        return(EXIT_FAILURE);
    }
Alen Hadzic
  • 29
  • 3
  • Probably should be `==`, as he does with `target`. But the `fclose` should not be there. – Karsten Koop Dec 13 '16 at 12:30
  • Yes, i saw that eventually :) – Alen Hadzic Dec 13 '16 at 12:43
  • my "cpydatabases.db" is not the same size ( 7KB the Databases, 2KB the cpydatabases)...the code works but i cant figure out what's going wrong in the &source memory address and why is it null – egoist Dec 13 '16 at 22:16
  • the two arrays: `source_file[]` and `target_file[]` are unused. the statement: `fclose( target );` should be saying: `fclose( source );` – user3629249 Dec 14 '16 at 15:14
  • yeap they are leftovers of the last update i did to the code...sorry for posting them – egoist Dec 14 '16 at 22:34
  • The reason why the cpydatabases file is 2KB is because you have set the maximum #BUFFERSIZE to "2048", which is 2KB. You should increase that if you want bigger files than 2KB. – Alen Hadzic Dec 15 '16 at 09:22
  • Instead using a define for size buffer, you could I guess calculate the size of the source file. For example like this: int calcSize(FILE *fp) { int prev = ftell(fp); fseek(fp, 0L, SEEK_END); int size = ftell(fp); fseek(fp, prev, SEEK_SET); //go back to where we were return size; } – Alen Hadzic Dec 15 '16 at 09:29
0

I think "Databases.db" is in not in same directory where executable is. You can give complete path of "Databases.db" or copy this file where your .sln file is.

Mohan
  • 1,871
  • 21
  • 34