Re: git: 7f0dc6e2cdfa - main - mkimg(1): process non-seekable output gracefully

On Tue, Mar 12, 2024 at 04:00:24PM +0000, Eugene Grosbein wrote:
> The branch main has been updated by eugen:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=7f0dc6e2cdfa0317c9917dd46e9da9d3897a8fbb
> 
> commit 7f0dc6e2cdfa0317c9917dd46e9da9d3897a8fbb
> Author:     Eugene Grosbein <eugen@FreeBSD.org>
> AuthorDate: 2024-03-12 15:55:42 +0000
> Commit:     Eugene Grosbein <eugen@FreeBSD.org>
> CommitDate: 2024-03-12 16:00:21 +0000
> 
>     mkimg(1): process non-seekable output gracefully
>     
>     mkimg may make severe load only to fail in the end
>     if output is non-seekable pipe, socket or FIFO
>     unless output format is raw disk image.
>     
>     Check it out and fail early. Make it clear in the manual.
>     
>     MFC after:      1 week
> ---
>  usr.bin/mkimg/mkimg.1 |  3 ++-
>  usr.bin/mkimg/mkimg.c | 14 +++++++++++++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/usr.bin/mkimg/mkimg.1 b/usr.bin/mkimg/mkimg.1
> index 820fb9ad1d5a..82bbee53a267 100644
> --- a/usr.bin/mkimg/mkimg.1
> +++ b/usr.bin/mkimg/mkimg.1
> @@ -22,7 +22,7 @@
>  .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>  .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  .\"
> -.Dd February 28, 2024
> +.Dd March 12, 2024
>  .Dt MKIMG 1
>  .Os
>  .Sh NAME
> @@ -64,6 +64,7 @@ The image file is a raw disk image by default, but the format of the
>  image file can be specified with the
>  .Ar format
>  argument.
> +Most formats require seekable output, except of raw disk image.
>  .Pp
>  The disk image can be made bootable by specifying the scheme-specific boot
>  block contents with the
> diff --git a/usr.bin/mkimg/mkimg.c b/usr.bin/mkimg/mkimg.c
> index 541d534f967f..3cd9b03c06e9 100644
> --- a/usr.bin/mkimg/mkimg.c
> +++ b/usr.bin/mkimg/mkimg.c
> @@ -555,6 +555,7 @@ mkimg(void)
>  int
>  main(int argc, char *argv[])
>  {
> +	const char *format_name;

const struct mkimg_format* format;

It seems weird to cache a single member.  For that matter, caching
anyting seems like overkill given than format_selected() just returns a
global variable's value.

>  	int bcfd, outfd;
>  	int c, error;
>  
> @@ -699,6 +700,7 @@ main(int argc, char *argv[])
>  			errc(EX_DATAERR, error, "boot code");
>  	}
>  
> +	format_name = format_selected()->name;
>  	if (verbose) {
>  		fprintf(stderr, "Logical sector size: %u\n", secsz);
>  		fprintf(stderr, "Physical block size: %u\n", blksz);
> @@ -709,10 +711,20 @@ main(int argc, char *argv[])
>  			fprintf(stderr, "Partitioning scheme: %s\n",
>  			    scheme_selected()->name);
>  		fprintf(stderr, "Output file format:  %s\n",
> -		    format_selected()->name);
> +		    format_name);
>  		fputc('\n', stderr);
>  	}
>  
> +#if defined(SPARSE_WRITE)
> +	/*
> +	 * sparse_write() fails if output is not seekable so fail early
> +	 * not wasting some load unless output format is raw
> +	 */
> +	if (strcmp("raw", format_name) &&
> +	    lseek(outfd, (off_t)0, SEEK_CUR) == -1 && errno == ESPIPE)
> +		errx(EX_USAGE, "%s: output must be seekable", format_name);
> +#endif

Maybe add a flag for non-seeking formats rather than hardcoding a
strcmp?

-- Brooks