Re: PATCH: Plug some fd leaks in bin_print

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

I have some doubts about some of this.  In fact I suspect that these are
mostly false-positives because the option letters tested in the if()
conditions preceding each of these goto's are (or should be) mutually
exclusive.

On Jan 7,  1:25am, Mikael Magnusson wrote:
}
}  	    if (*eptr) {
} -		zwarnnam(name, "number expcted after -%c: %s", 'C', argptr);
} -		return 1;
} +		zwarnnam(name, "number expected after -%c: %s", 'C', argptr);
} +		ret = 1;
} +		goto out_print;
}  	    }
}  	    if (nc <= 0) {
}  		zwarnnam(name, "invalid number of columns: %s", argptr);
} -		return 1;
} +		ret = 1;
} +		goto out_print;
}  	    }

I'm not sure what coverity had to say here, but these can't be right.  The
descriptor should NOT be fflush()d on this error.

}  	    queue_signals();
}  	    zpushnode(bufstack, sepjoin(args, NULL, 0));
}  	    unqueue_signals();
} -	    return 0;
} +	    goto out_print;

This can't be right either; there shouldn't be any output when pushing on
the buffer stack, so again we shouldn't be fflush()ing.


}  			zwarnnam(name, "option -S takes a single argument");
} -			return 1;
} +			ret = 1;
} +			goto out_print;

Same complaint.

}      if (OPT_ISSET(ops,'z') || OPT_ISSET(ops,'s')) {
} +	if (fout != stdout)
} +	    fclose(fout);
} +	fout = stdout;

Why is that needed when the very next couple of lines are going to over-
write fout with a different open?

I'm just going to quit complaining about the rest of these; I think
they're all incorrect.  If nothing else, they shouldn't introduce the
possibility of printing BOTH an argument-parsing error AND a write-
failed error.

This needs to be looked at more closely.

-- 
Barton E. Schaefer