Re: [PATCH v2 1/3] diff-parseopt: correct variable types that are used by parseopt

[Todd Zullinger <tmz@pobox.com>]

Nguyễn Thái Ngọc Duy wrote:
> Most number-related OPT_ macros store the value in an 'int'
> variable. Many of the variables in 'struct diff_options' have a
> different type, but during the conversion to using parse_options() I
> failed to notice and correct.
> 
> The problem was reported on s360x which is a big-endian
> architechture. The variable to store '-w' option in this case is
> xdl_opts, 'long' type, 8 bytes. But since parse_options() assumes
> 'int' (4 bytes), it will store bits in the wrong part of xdl_opts. The
> problem was found on little-endian platforms because parse_options()
> will accidentally store at the right part of xdl_opts.
> 
> There aren't much to say about the type change (except that 'int' for
> xdl_opts should still be big enough, since Windows' long is the same
> size as 'int' and nobody has complained so far). Some safety checks may
> be implemented in the future to prevent class of bugs.
> 
> Reported-by: Todd Zullinger <tmz@pobox.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  diff.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/diff.h b/diff.h
> index b20cbcc091..d5e44baa96 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -169,7 +169,7 @@ struct diff_options {
>  	const char *prefix;
>  	int prefix_length;
>  	const char *stat_sep;
> -	long xdl_opts;
> +	int xdl_opts;
>  
>  	/* see Documentation/diff-options.txt */
>  	char **anchors;

FWIW, I ran this versions of the series through the fedora
buildsystem and noticed no issues on s390x or any other
architectures.

Thanks,

-- 
Todd