6

[Question 1]

When I open a file into a function, generally I do something like this:

int read_file (char *filename)
{
    FILE *fin;

    if ( !(fin = fopen(filename, "r")) )
        return 1;

    /* ... */

    return fclose(fin);
}

int main ()
{
    char filename[100];

    if ( read_file(filename) )
    {
        perror(filename);
        exit(1);
    }

    return 0;
}

Generally 0 return value is for errors (right?) then I can change the previous code into:

int read_file (char *filename)
{
    FILE *fin;

    if ( !(fin = fopen(filename, "r")) )
        return 0;

    /* ... */

    return !fclose(fin);
}

int main ()
{
    char filename[100];

    if ( !read_file(filename) )
    {
        perror(filename);
        exit(1);
    }

    return 0;
}

But I think that the first code is more clean.

Another option is only change return 1; into return -1; (in the first code that I wrote).

What's the best version?

[Question 2]

If I must handle more errors, is it correct a code like this?

int read_file (char *filename, int **vet)
{
    FILE *fin;

    if ( !(fin = fopen(filename, "r")) )
    {
        perror(filename);
        return 1;
    }

    * vet = malloc (10 * sizeof(int));
    if ( *vet == NULL )
    {
        perror("Memory allocation error.\n");
        return 1;   
    }

    /* ... */

    return fclose(fin);
}

int main ()
{
    char filename[100];
    int *vet;

    if ( read_file(filename, &vet) )
        exit(1);

    return 0;
}
ᴜsᴇʀ
  • 1,109
  • 2
  • 9
  • 23
  • will not `(fin = fopen(filename, "r")` be always true? – Dipto Jan 21 '14 at 19:59
  • 2
    In general we use `0` as the default error return as `0` refers to `false` and non null values refers to `true`. But when different errors needs to be handled we use negative values for errors. In your second question it's better to return different values for different errors to be able to know exactly which error caused the fail. – rullof Jan 21 '14 at 20:02
  • It's more customary in C to use a negative value as error indicator and zero or a positive number for success. In your function, zero or a positive return value could indicate the number of `vec`s that were read, where zero means, the file is okay but didn't have any `vet`s in it. I like the idea of using different negative values (maybe `enum`s) to indicate different errors. – M Oehm Jan 21 '14 at 20:23
  • Your comment that 1 is "generally for errors" is probably referring to program exit codes. Generally exit(0) refers to successful execution and any other value indicates the program exited with some kind of error. Functions within the program are free to use whatever scheme is convenient to signal errors. For example, some people like to use setjmp longjmp for this purpose – Brandin Jan 21 '14 at 20:29
  • 2
    @Dipto `If the file is successfully opened, the function returns a pointer to a FILE object that can be used to identify the stream on future operations. Otherwise, a null pointer is returned.`[[cplusplus.com](http://www.cplusplus.com/reference/cstdio/fopen/)] – ᴜsᴇʀ Jan 21 '14 at 21:40
  • @user, yes, But you are assigning that returned `FILE pointer` to `fin`, and the result of this assignment is always true. as in case `int a; (a=5)` is always true. If the file is successfully opened, the statement will be `if(fin=)` other wise it will be `if(fin=NULL)`, will not both be true? – Dipto Jan 22 '14 at 07:12
  • 1
    @Dipto No, as you say _"other wise it will be `if(fin=NULL)`"_, then in this case it's false ([link](http://stackoverflow.com/a/9894108/3185964)). – ᴜsᴇʀ Jan 22 '14 at 10:27
  • @user, yes.. I was arguing about a totally different thing (I was stupid to think like that). This is correct. though I have no confusion about `NULL`. – Dipto Jan 22 '14 at 10:33

2 Answers2

4

Re Q1:

a) Most POSIX functions actually return -1 (or <0) for errors, not 0. Look at (for instance) open(), close(), read(), write() and so forth. The exception is the POSIX calls that return pointers, e.g. fopen(), which returns a FILE *. These return NULL on error.

b) I code my code to work like POSIX functions, which is similar the innards of many linux programs. I would call this 'the UNIX C standard'. However, many C++ programs and Java programs use true for success and false for failure. When these programmers move to C, they use 1 for success, and 0 for failure. This isn't wrong, but does cause confusion (well, causes me confusion). The worst result is when both standards are used in the same program. Picking a standard and sticking to it is more important than which standard you choose.

c) My own choice (in relation to Q1), would be to return -1 on error (i.e. as per your 'another choice' line).

Re Q2: mostly right, yes.

a) If your program is successful, better to exit(0) than return 0 I believe.

b) Quite where you perror is up to you. Perhaps you want to print the error in main().

c) Using perror immediately followed by exit(1) (or perhaps a different exit code depending on the error) is reasonable normal if you have no clean up to do or clean up within atexit.

d) If you are returning the result of fclose() on error, then the return if fopen fails should be -1 (or EOF) not 1 as if fclose() fails it returns EOF (otherwise known as -1).

e) Nit: your main function should have parameters (e.g. int main(char **argv, int argc))

abligh
  • 24,573
  • 4
  • 47
  • 84
  • 2
    **(Q2b)** _"Quite where you `perror` is up to you. Perhaps you want to print the error in `main()`."_ But if I use `perror` into `main()` I can't differentiate the errors! **(Q2d)** _"If you are returning the result of `fclose()` on error, then the return if `fopen` fails should be `-1` (or `EOF`) not 1 as if `fclose()` fails it returns `EOF` (otherwise known as `-1`)."_ But if `fopen` fails it returns to `main()`: it not arrive until the `fclose()`. – ᴜsᴇʀ Jan 21 '14 at 21:34
  • 1
    **(Q2e)** _"Nit: your main function should have parameters (e.g. `int main(char **argv, int argc))`"_ If you don't use the `main()` parameters you can not write them. From Standard documents 3.6.1.2 Main Function: _"It shall have a return type of type int, but otherwise its type is implementation-defined. All implementations shall allow both of the following definitions of main: `int main() { / ... / }` and `int main(int argc, char* argv[]) { / ... / }`"_ – ᴜsᴇʀ Jan 21 '14 at 21:51
0

in respect of negative numbers for errors - cppcheck gives warnings for that. Choosing a standard within a program suite is a good idea - programmers have enough internal logic to deal with without duplicating ... So after trying to fix a few FOSS programs I am likely to go with cppcheck recommendations then at least I can have something check my adopted standard.

matrixmike
  • 31
  • 3