git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	bturner@atlassian.com, tmz@pobox.com,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 1/3] diff-parseopt: correct variable types that are used by parseopt
Date: Fri, 24 May 2019 16:24:40 +0700	[thread overview]
Message-ID: <20190524092442.701-2-pclouds@gmail.com> (raw)
In-Reply-To: <20190524092442.701-1-pclouds@gmail.com>

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 | 70 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/diff.h b/diff.h
index b20cbcc091..4527daf6b7 100644
--- a/diff.h
+++ b/diff.h
@@ -65,39 +65,39 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 
 #define DIFF_FLAGS_INIT { 0 }
 struct diff_flags {
-	unsigned recursive;
-	unsigned tree_in_recursive;
-	unsigned binary;
-	unsigned text;
-	unsigned full_index;
-	unsigned silent_on_remove;
-	unsigned find_copies_harder;
-	unsigned follow_renames;
-	unsigned rename_empty;
-	unsigned has_changes;
-	unsigned quick;
-	unsigned no_index;
-	unsigned allow_external;
-	unsigned exit_with_status;
-	unsigned reverse_diff;
-	unsigned check_failed;
-	unsigned relative_name;
-	unsigned ignore_submodules;
-	unsigned dirstat_cumulative;
-	unsigned dirstat_by_file;
-	unsigned allow_textconv;
-	unsigned textconv_set_via_cmdline;
-	unsigned diff_from_contents;
-	unsigned dirty_submodules;
-	unsigned ignore_untracked_in_submodules;
-	unsigned ignore_dirty_submodules;
-	unsigned override_submodule_config;
-	unsigned dirstat_by_line;
-	unsigned funccontext;
-	unsigned default_follow_renames;
-	unsigned stat_with_summary;
-	unsigned suppress_diff_headers;
-	unsigned dual_color_diffed_diffs;
+	unsigned int recursive;
+	unsigned int tree_in_recursive;
+	unsigned int binary;
+	unsigned int text;
+	unsigned int full_index;
+	unsigned int silent_on_remove;
+	unsigned int find_copies_harder;
+	unsigned int follow_renames;
+	unsigned int rename_empty;
+	unsigned int has_changes;
+	unsigned int quick;
+	unsigned int no_index;
+	unsigned int allow_external;
+	unsigned int exit_with_status;
+	unsigned int reverse_diff;
+	unsigned int check_failed;
+	unsigned int relative_name;
+	unsigned int ignore_submodules;
+	unsigned int dirstat_cumulative;
+	unsigned int dirstat_by_file;
+	unsigned int allow_textconv;
+	unsigned int textconv_set_via_cmdline;
+	unsigned int diff_from_contents;
+	unsigned int dirty_submodules;
+	unsigned int ignore_untracked_in_submodules;
+	unsigned int ignore_dirty_submodules;
+	unsigned int override_submodule_config;
+	unsigned int dirstat_by_line;
+	unsigned int funccontext;
+	unsigned int default_follow_renames;
+	unsigned int stat_with_summary;
+	unsigned int suppress_diff_headers;
+	unsigned int dual_color_diffed_diffs;
 };
 
 static inline void diff_flags_or(struct diff_flags *a,
@@ -151,7 +151,7 @@ struct diff_options {
 	int skip_stat_unmatch;
 	int line_termination;
 	int output_format;
-	unsigned pickaxe_opts;
+	unsigned int pickaxe_opts;
 	int rename_score;
 	int rename_limit;
 	int needed_rename_limit;
@@ -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;
-- 
2.22.0.rc0.322.g2b0371e29a


  reply	other threads:[~2019-05-24  9:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24  9:24 [PATCH 0/3] fix diff-parseopt regressions Nguyễn Thái Ngọc Duy
2019-05-24  9:24 ` Nguyễn Thái Ngọc Duy [this message]
2019-05-28 19:23   ` [PATCH 1/3] diff-parseopt: correct variable types that are used by parseopt Junio C Hamano
2019-05-24  9:24 ` [PATCH 2/3] diff-parseopt: restore -U (no argument) behavior Nguyễn Thái Ngọc Duy
2019-05-24  9:24 ` [PATCH 3/3] parse-options: check empty value in OPT_INTEGER and OPT_ABBREV Nguyễn Thái Ngọc Duy
2019-05-24  9:36 ` [PATCH 4/3] parse-options: make compiler check value type mismatch Nguyễn Thái Ngọc Duy
2019-05-24 17:36 ` [PATCH 0/3] fix diff-parseopt regressions Todd Zullinger
2019-05-24 20:58   ` Todd Zullinger
2019-05-25 10:22   ` Duy Nguyen
2019-05-25 20:48     ` Todd Zullinger
2019-05-29  9:11 ` [PATCH v2 " Nguyễn Thái Ngọc Duy
2019-05-29  9:11   ` [PATCH v2 1/3] diff-parseopt: correct variable types that are used by parseopt Nguyễn Thái Ngọc Duy
2019-05-29 16:43     ` Eric Sunshine
2019-05-29 16:47     ` Todd Zullinger
2019-05-29 19:54       ` Junio C Hamano
2019-05-29  9:11   ` [PATCH v2 2/3] diff-parseopt: restore -U (no argument) behavior Nguyễn Thái Ngọc Duy
2019-05-29  9:11   ` [PATCH v2 3/3] parse-options: check empty value in OPT_INTEGER and OPT_ABBREV Nguyễn Thái Ngọc Duy
2019-05-29 18:03   ` [PATCH v2 0/3] fix diff-parseopt regressions Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190524092442.701-2-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=avarab@gmail.com \
    --cc=bturner@atlassian.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tmz@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).