Re: [PATCH 4/5 v2] set: check seek success

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

On Sat, Nov 08, 2014 at 11:14:09PM +0100, Patrick Georgi wrote:
> This could silently fail which leads to surprising behaviour.
> 
> Found-by: Coverity Scan
> Signed-off-by: Patrick Georgi <patrick-U1IrdZU7gScKt1dFYRw6Gg@public.gmane.org>
> ---
>  src/set.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/set.c b/src/set.c
> index ff32b53..907d640 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -59,7 +59,11 @@ read_from_image(char	*filename,
>  		return result;
>  	}
>  
> -	fseek(fp, offset, SEEK_SET);
> +	if (fseek(fp, offset, SEEK_SET) == -1) {
> +		printf("Error: Couldn't seek to %s(%d)\n", filename, offset);

offset is unsigned, so the format specifier should be %u. I also wonder
whether it would be good to output errno along with the message (or the
string representation thereof) to increase the information content. But
given that none of the other error messages have that it could be a
separate patch.

With the %d -> %u for the format specifier, this is:

Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]