#35 lnewusers: flose(fp) to avoid a Coverity warning
Opened 11 months ago by jhrozek. Modified 11 months ago
jhrozek/libuser lnewuser_fclose  into  master

file modified
+4

@@ -305,6 +305,10 @@ 

  		lu_ent_clear_all(groupEnt);

  	}

  

+         if (fp) {

+             fclose(fp);

+         }

+ 

  	lu_ent_free(groupEnt);

  	lu_ent_free(ent);

  

Merges:
https://pagure.io/libuser/issue/20

Please note that this patch also ADDS a Coverity warning:

libuser-0.62/apps/lnewusers.c:97: deref_ptr_in_call: Dereferencing pointer "fp". [Note: The source code implementation of the function has been overridden by a builtin model.]
libuser-0.62/apps/lnewusers.c:306: check_after_deref: Null-checking "fp" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
#  304|     }
#  305|   
#  306|->         if (fp) {
#  307|               fclose(fp);
#  308|           }

But I think checking a file handle validity is a good defensive behaviour.

fclose(stdin) causes undefined behavior, see http://pubs.opengroup.org/onlinepubs/9699919799/functions/fclose.html ,

(My personal taste would be not to add “defensive” tests that can’t happen, but that’s neither here nor there; it’s your project now.)

Yes, I tend to agree.

I'm actually tempted to just close this PR and the issue. This is a short-running process after all, not a daemon.

Or, well, be precise, and have a FILE *file_to_close, which is NULL if input is from stdin, and non-NULL if it is another handle. That might confuse Coverity even more, though…

Metadata