On Thu, Jan 31, 2019 at 2:22 PM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> Signed-off-by: Petr Lautrbach <plautrba@xxxxxxxxxx>
> ---
> libsemanage/src/direct_api.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index c58961be..8e4d116d 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -1028,7 +1028,7 @@ static int semanage_direct_write_langext(semanage_handle_t *sh,
>
> fp = NULL;
>
> - ret = 0;
> + return 0;
>
> cleanup:
> if (fp != NULL) fclose(fp);
> @@ -2177,7 +2177,6 @@ cleanup:
> semanage_module_info_destroy(sh, modinfo);
> free(modinfo);
>
> - if (fp != NULL) fclose(fp);
> return status;
> }
>
> @@ -2342,16 +2341,6 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
> free(tmp);
> tmp = NULL;
>
> - if (fclose(fp) != 0) {
> - ERR(sh,
> - "Unable to close %s module lang ext file.",
> - (*modinfo)->name);
> - status = -1;
> - goto cleanup;
> - }
> -
> - fp = NULL;
> -
> /* lookup enabled/disabled status */
> ret = semanage_module_get_path(sh,
> *modinfo,
> @@ -2395,7 +2384,13 @@ cleanup:
> free(modinfos);
> }
>
> - if (fp != NULL) fclose(fp);
> + if (fp != NULL && fclose(fp) != 0) {
> + ERR(sh,
> + "Unable to close %s module lang ext file.",
> + (*modinfo)->name);
> + status = -1;
> + }
> +
> return status;
> }
>
> --
> 2.20.1
After reading this patch, I am wondering what the USE_AFTER_FREE
defects were about. Were there real use-after-fclose bugs that are
fixed by this patch? Or is this patch more about silencing Coverity's
false positives due to difficulties in parsing constructions such as
"fclose(fp);fp = NULL; if(f != NULL)fclose(fp);"? It would be useful
if the patch description contains answers to these questions.
Otherwise the content of the patch looks good to me.
Nicolas
