r3.in.v5d: Fix unchecked return value

[ShubhamDesai]

This pull request addresses an issue identified by Coverity Scan (CID: 1207300), where the return value of the lseek function is not checked.

[wenzeslaus]

If that fails, more is needed than a simple printf and moving to the next line. See how lseek failures are handled elsewhere in the code.

[ShubhamDesai]

Thanks, i will refer those codes.

[ShubhamDesai]

referred the approach from this file : lib/segment/seek.c
errorno and G_fatal_error is used to log the error message

[nilason]

For some reason the raster3d library has partly its own error handling (API). I'm not familiar with the motivation for treating raster3d lib differently. In any case, by that logic it should be Rast3d_fatal_error().

However, in this module all functions calling lseek returns 0 by error, which is then handled accordingly in r3.in.v5d/main.c (although that may need some care, but that is out of scope for this PR). So, I'd suggest (1) to address all calls of lseek() in the r3.in.v5d/v5d.c file and (2) change (all related) printf() to G_warning() and return 0;.

(You could update SKIP(N) to a multiline macro with a do-while statement.)

[wenzeslaus]

I agree with @nilason. The function is written to behave that way, so that of course wins over the general practice.

[ShubhamDesai]

For some reason the raster3d library has partly its own error handling (API). I'm not familiar with the motivation for treating raster3d lib differently. In any case, by that logic it should be Rast3d_fatal_error().

However, in this module all functions calling lseek returns 0 by error, which is then handled accordingly in r3.in.v5d/main.c (although that may need some care, but that is out of scope for this PR). So, I'd suggest (1) to address all calls of lseek() in the r3.in.v5d/v5d.c file and (2) change (all related) printf() to G_warning() and return 0;.

(You could update SKIP(N) to a multiline macro with a do-while statement.)

@nilason just one doubt about the second change that you suggested all printf to change to G_warning its related to the lseek printfs right and also should it be G_warning or G_fatal

[nilason]

@nilason just one doubt about the second change that you suggested all printf to change to G_warning its related to the lseek printfs right and also should it be G_warning or G_fatal

What I mean is to address all (but one) lseek in this file. I believe there are 14 cases. 12 of these are in functions returning 0 in case of failure, one is returning the lseek return value, and one is part of a macro.
In all these cases (except in ltell() which you should leave as is) you can check if lseek gives -1 (or < 0) and if that's the case, issue G_warning (no G_fatal_error) and return 0. There are two cases with failing lseek giving printf "error" message, which you can change to G_warning.

[ShubhamDesai]

I have done the changes. Could you please look into it once also please tell me if the SKIP(N) macro is correctly updated. I don't know whether my interpretation about it is correct or not.

[nilason]

Looks good, the macro too!
The printf messages were never translated, so let’s use our “standard” text there also. strerror is more informative anyway.