r.out.png: fix consecutive fclose calls on same pointer

[ymdatta]

This patch fixes two issues:

1. In one of the code paths, we are calling fclose on a file pointer which could potentially be NULL. Doing that would lead to undefined behavior. Check if a file pointer is NULL before closing it.

2. If we call fclose on same file pointer twice, in the second instance we could be closing file descriptor allocated to some other file, which typically happens to a freed descriptor.

This issue was found by using cppcheck tool.

Additional information:

1. Machine used: Virtual Machine running Ubuntu 22.04.4 LTS.
2. Tool used in finding this error: cppcheck

Tool's output after applying the patch

3. Reproduction rate: 100%, reproducible every time.
4. Architecture: Not specific to any underlying architecture.

[echoix]

I agree with the PR. I would want some feedback on if it is the correct pattern to be autonomous on future reviews of this kind.

[wenzeslaus]

Here I my thoughts:

• Default value is set.
• NULL is the right default for FILE.
• fclose requires a check.

What Cppcheck does not see is that G_fatal_error does not return, so the later fclose calls will not happen. I'm not sure what is better here, try to configure, try to provide attributes to the function, or simply apply the best practice (close and set to NULL) regardless of the fact that it is not need. I lean towards the best practice as it is seems the simplest, however providing more info about the function to the compilers or the linters seems good long-term strategy.

[nilason]

Missed this by the second. The only place where it would have made any (if ever so little) sense to reset fp = NULL was

grass/raster/r.out.png/main.c

Lines 372 to 373 in 917ba58

	if (fp)
	fclose(fp);

.

Other than that note, this was good to go.

[wenzeslaus]

The last fp = NULL would indeed fit the pattern.

[ymdatta]

I will push another small patch adding fp = NULL at the missed place.