From patchwork Fri May 24 09:24:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Duy Nguyen X-Patchwork-Id: 10959583 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 578A11395 for ; Fri, 24 May 2019 09:24:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4502128868 for ; Fri, 24 May 2019 09:24:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3314828807; Fri, 24 May 2019 09:24:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0E13128807 for ; Fri, 24 May 2019 09:24:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390100AbfEXJY5 (ORCPT ); Fri, 24 May 2019 05:24:57 -0400 Received: from mail-pg1-f196.google.com ([209.85.215.196]:40801 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389787AbfEXJY4 (ORCPT ); Fri, 24 May 2019 05:24:56 -0400 Received: by mail-pg1-f196.google.com with SMTP id d30so4740858pgm.7 for ; Fri, 24 May 2019 02:24:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=d2wOxNCt+FPNfebWjxvOuoSawQQz8cpz65cxOkj61I0=; b=K40vTBethlrdvkffpMirpm5L54tKX4j+TKt1vf+btESFbm/vrk0dwY7okxpZI4XcFa GOFgD/x3701a120I389EuwgffY5m1f6si/ZzpJP77exIrIb6AFTI142xMZ5ut2KQ+zGU +UiNMUchXoMTAyTJkefpmwn1kedYFDyVfCQI9kR+ezODdiBvis6DD9T/9uYq/ZJB/KjS YTmKLJahxHU6LajXFI6VjtoyrF0/r2E432Gk91Rlv1gfDrG+acLMjbGr9WcK7P6CvLmd s1hS1LtTf6NP0DbuZG5W9Bt3QUq/w1G9B+RG5HJ3sn7Ub0OdV2QH8azNIB9uHqWCPnXz +cQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=d2wOxNCt+FPNfebWjxvOuoSawQQz8cpz65cxOkj61I0=; b=hIdY+GDAHpWwY/TJaixRalGabSq53HsLkmUaNOCJwmyraycvfKwlV2jSf0OEu6Rq3w 0YqH4mn4l4Y5gptaav8UFPi1XHd2mAUF3V4qKIi/Tmfk7NL3loBCMbr4V1mNw61fjj7+ oKeq6jzAY9KA3CORGCg20KJJSGw/u6KJlGuvTyRcWNg0R+eViRMxfxGweC0GRP5i5avB n1NrbusXaA5XAaNalChD/WdSv3knMJRfwXYyczWoTQkAiyvRwmvA4ljyckTSVIa2TzDm nW6gw0p0XaXQSkOpu8JTPrW57L1Rb0DEX4SoEyAIfvBG+YCgSFg1OjnM07r1NvoqqdB1 CQfw== X-Gm-Message-State: APjAAAXQKyhMsVcL61ewAZi1a7QgrysTOZFk28+aoaL3k3HzrWPu3iKY yu/HE82DSZ9ae9T6h8Ga6KBpk2eO X-Google-Smtp-Source: APXvYqyLtAjEQcOzq01iUT1PxI/0EqNRgp7Uv6EY2/s8UGAkPrUryjd0GpOlvJc+W4kKEmCpLDogdQ== X-Received: by 2002:a17:90a:35c:: with SMTP id 28mr7366895pjf.110.1558689895406; Fri, 24 May 2019 02:24:55 -0700 (PDT) Received: from ash ([115.72.28.243]) by smtp.gmail.com with ESMTPSA id 4sm3072854pfj.111.2019.05.24.02.24.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 02:24:54 -0700 (PDT) Received: by ash (sSMTP sendmail emulation); Fri, 24 May 2019 16:24:50 +0700 From: =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= To: git@vger.kernel.org Cc: Junio C Hamano , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= , bturner@atlassian.com, tmz@pobox.com, =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= Subject: [PATCH 1/3] diff-parseopt: correct variable types that are used by parseopt Date: Fri, 24 May 2019 16:24:40 +0700 Message-Id: <20190524092442.701-2-pclouds@gmail.com> X-Mailer: git-send-email 2.22.0.rc0.322.g2b0371e29a In-Reply-To: <20190524092442.701-1-pclouds@gmail.com> References: <20190524092442.701-1-pclouds@gmail.com> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Nguyễn Thái Ngọc Duy --- 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; From patchwork Fri May 24 09:24:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Duy Nguyen X-Patchwork-Id: 10959585 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B50F513AD for ; Fri, 24 May 2019 09:25:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A13A728807 for ; Fri, 24 May 2019 09:25:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9237728867; Fri, 24 May 2019 09:25:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F34E928807 for ; Fri, 24 May 2019 09:25:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390114AbfEXJZC (ORCPT ); Fri, 24 May 2019 05:25:02 -0400 Received: from mail-pf1-f195.google.com ([209.85.210.195]:37448 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389777AbfEXJZB (ORCPT ); Fri, 24 May 2019 05:25:01 -0400 Received: by mail-pf1-f195.google.com with SMTP id a23so4990636pff.4 for ; Fri, 24 May 2019 02:25:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ZkU0Sy3kGo2Tpxjx47UMjQZq7d/ezG/8c+Qx3KTnaGM=; b=a8C8fY1yhBsEIW2pYCqHNdn8GcIukZzcqB0ucnl/7l/0oXhvLdg1yRdWulKWxz7FvH 0o8zMDgBlCHHTI5Zz3i2HZQewA7RML2aw+aqM9WDWDYf6w2n0RiaZfEbaDm8oN3KDer+ nzoGma1cJekO8mWivYZlWS1WKicFFOV7n5nstjvDPm9875h767bXwFuvifgiWpPRj706 I/tqrGJXn1tEx6hI2LthJYXwO0A6l1EAdKCZk9cqWZ2tGJl5iABDbiJeqDqiY2zLdnnO A7eZbPLOW18rvFWhmVlm576o2GQWJ742XZRh1bJOBLUiWBrjwNceKZh4cZgg83jZ5Cur eqig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ZkU0Sy3kGo2Tpxjx47UMjQZq7d/ezG/8c+Qx3KTnaGM=; b=FBpg2M2wrgGCP1UEFGls+Zi0a99x0s/s9EZROqSu1Lk+u55ZI8TGp41T7oETLSx/PO XCReWt2V7g4kqmKWVTc/i+uf5wla1vQvWTF/OWEix6aNV+bxa22yNnndO1exMam00u2y YcXJwn1xxy6io2wiHkwIwJearLHM7mbVQ7RDW41Ie8hRZAjlfEWMbrb68fIjS4IIQrlD hwXV3IB4lxiS9e2VclxgfEHxONB1rU+dDX162xyLE7Q7C48DaNfiMKQZa3WB/g3fQUdy RnaXoik9EgauUqn0lUNo6ew7pI57koeGgLtGSmOkmD+ATXXTAjs3cY1YPbTQj9VMbHa8 LacA== X-Gm-Message-State: APjAAAVIRo5/6Zbo6dZnrtSo3sEeGCT3nDj3AzqoGhlYgB44RmMNPaDX HOHuDMZ70Ofse3Jz34lbJ32bIJVH X-Google-Smtp-Source: APXvYqwRVBc1+ZQuPCJcpH6OXS2cv5vnjxChpMmjww088eursLi4ohB1w43cuWZuEsH2raGoNEOzJA== X-Received: by 2002:a17:90a:32c1:: with SMTP id l59mr7650060pjb.1.1558689900303; Fri, 24 May 2019 02:25:00 -0700 (PDT) Received: from ash ([115.72.28.243]) by smtp.gmail.com with ESMTPSA id x18sm3823972pfj.17.2019.05.24.02.24.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 02:24:59 -0700 (PDT) Received: by ash (sSMTP sendmail emulation); Fri, 24 May 2019 16:24:55 +0700 From: =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= To: git@vger.kernel.org Cc: Junio C Hamano , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= , bturner@atlassian.com, tmz@pobox.com, =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= Subject: [PATCH 2/3] diff-parseopt: restore -U (no argument) behavior Date: Fri, 24 May 2019 16:24:41 +0700 Message-Id: <20190524092442.701-3-pclouds@gmail.com> X-Mailer: git-send-email 2.22.0.rc0.322.g2b0371e29a In-Reply-To: <20190524092442.701-1-pclouds@gmail.com> References: <20190524092442.701-1-pclouds@gmail.com> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Before d473e2e0e8 (diff.c: convert -U|--unified, 2019-01-27), -U and --unified are implemented with a custom parser opt_arg() in diff.c. I didn't check this code carefully and not realize that it's the equivalent of PARSE_OPT_NONEG | PARSE_OPT_OPTARG. In other words, if -U is specified without any argument, the option should be accepted, and the default value should be used. Without PARSE_OPT_OPTARG, parse_options() will reject this case and cause a regression. Reported-by: Bryan Turner Signed-off-by: Nguyễn Thái Ngọc Duy --- diff.c | 10 ++++--- t/t4013-diff-various.sh | 2 ++ t/t4013/diff.diff_-U1_initial..side (new) | 29 ++++++++++++++++++++ t/t4013/diff.diff_-U2_initial..side (new) | 31 ++++++++++++++++++++++ t/t4013/diff.diff_-U_initial..side (new) | 32 +++++++++++++++++++++++ 5 files changed, 100 insertions(+), 4 deletions(-) diff --git a/diff.c b/diff.c index 4d3cf83a27..80ddc11671 100644 --- a/diff.c +++ b/diff.c @@ -5211,9 +5211,11 @@ static int diff_opt_unified(const struct option *opt, BUG_ON_OPT_NEG(unset); - options->context = strtol(arg, &s, 10); - if (*s) - return error(_("%s expects a numerical value"), "--unified"); + if (arg) { + options->context = strtol(arg, &s, 10); + if (*s) + return error(_("%s expects a numerical value"), "--unified"); + } enable_patch_output(&options->output_format); return 0; @@ -5272,7 +5274,7 @@ static void prep_parse_options(struct diff_options *options) DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT), OPT_CALLBACK_F('U', "unified", options, N_(""), N_("generate diffs with lines context"), - PARSE_OPT_NONEG, diff_opt_unified), + PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified), OPT_BOOL('W', "function-context", &options->flags.funccontext, N_("generate diffs with lines context")), OPT_BIT_F(0, "raw", &options->output_format, diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 9f8f0e84ad..a9054d2db1 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -338,6 +338,8 @@ format-patch --inline --stdout initial..master^^ format-patch --stdout --cover-letter -n initial..master^ diff --abbrev initial..side +diff -U initial..side +diff -U1 initial..side diff -r initial..side diff --stat initial..side diff -r --stat initial..side diff --git a/t/t4013/diff.diff_-U1_initial..side b/t/t4013/diff.diff_-U1_initial..side new file mode 100644 index 0000000000..b69f8f048a --- /dev/null +++ b/t/t4013/diff.diff_-U1_initial..side @@ -0,0 +1,29 @@ +$ git diff -U1 initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -2 +2,3 @@ A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -3 +3,4 @@ + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.diff_-U2_initial..side b/t/t4013/diff.diff_-U2_initial..side new file mode 100644 index 0000000000..8ffe04f203 --- /dev/null +++ b/t/t4013/diff.diff_-U2_initial..side @@ -0,0 +1,31 @@ +$ git diff -U2 initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -2,2 +2,5 @@ + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.diff_-U_initial..side b/t/t4013/diff.diff_-U_initial..side new file mode 100644 index 0000000000..c66c0dd5c6 --- /dev/null +++ b/t/t4013/diff.diff_-U_initial..side @@ -0,0 +1,32 @@ +$ git diff -U initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -1,3 +1,6 @@ + 1 + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ From patchwork Fri May 24 09:24:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Duy Nguyen X-Patchwork-Id: 10959587 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CCA6D1395 for ; Fri, 24 May 2019 09:25:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B852928807 for ; Fri, 24 May 2019 09:25:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id ACAEF287E3; Fri, 24 May 2019 09:25:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C0D7D287E3 for ; Fri, 24 May 2019 09:25:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390137AbfEXJZG (ORCPT ); Fri, 24 May 2019 05:25:06 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:41818 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390120AbfEXJZG (ORCPT ); Fri, 24 May 2019 05:25:06 -0400 Received: by mail-pl1-f193.google.com with SMTP id f12so3929244plt.8 for ; Fri, 24 May 2019 02:25:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=C7hX91qz3vqJ6SvsPoUxKHsJMUdb/xZv58Ut8nEe1vU=; b=vPWp2GFwtdeerOSRZSZ6M8685vxPdHRj5FCPHfjmhQQNo9Xu9wOP/IwAHdJawIjZ1m YWy0Tksj1n/MFvG6MlHja7/mWBLAq8XMPnU2kyNuWiaXSPtjSznNOsKrQ/VxOGkw68H+ cgr4A6hBU+3Fkp35c403Mfurrkghv0uBljl+nUiOgotGjZS4y3X8EkisA90c/6vHH9fU CvKgj1Th+sJAgdkorBEQ5boNy/xEL+KmSTJfr/T0SnUkaSNNIm9CgOZvO+S3Ot/GAPw+ hEzFJ7c87AFH9/ZnxaY9TNTMHluR1ZnP5Zjm+Ae0iLh/57cpFyZd9zWqM+bgP+nntN7I Q6Yg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=C7hX91qz3vqJ6SvsPoUxKHsJMUdb/xZv58Ut8nEe1vU=; b=B70IlxxkJQDMm5xRvOwOK+cAHSbyEMNWqlFUJYUgpkx6N0snu1lo/6y4KEKp3uP4In Yd8UYtPyeNLEnVWVjP/d/LYID2/4k2M7UxcQwj32PkYaimUUuKaVJUK+jc1lOX0KJg1P JX1oy4H7CuWzBqxvYnGrTDCyynSoft4+AKMVVhOWc+W1RYJNB6YuOVNBPf7sc+x5mFiA 8iupN8t5/yjBkzlU85px+NQxYshuJP3Z4yfrjsDWLYIzyEdo3UZLNJAJyC2xosjAL+jY Ea64sr9jtrhpILOajgpA9EmJGnEtBkFmBREYO2vHwlqzYVXo3fA9EoKysYhiqCEIFHnJ 4pFA== X-Gm-Message-State: APjAAAUYSMVKYVMwbtWz7Nrv3keGinatuDCvDPDzWJFIkbp/yluu//ln PpZ7gFlAzpA8hKhuA4ZOm0YHexlu X-Google-Smtp-Source: APXvYqzTabwpvc/IP1fZjpq2zUI0kSjKrrysWRXJMumt3gm6gXZlAv9qJjNPwq3xsihXcCiK2vNvhw== X-Received: by 2002:a17:902:b606:: with SMTP id b6mr106560990pls.100.1558689905557; Fri, 24 May 2019 02:25:05 -0700 (PDT) Received: from ash ([115.72.28.243]) by smtp.gmail.com with ESMTPSA id h62sm1819671pgc.77.2019.05.24.02.25.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 02:25:04 -0700 (PDT) Received: by ash (sSMTP sendmail emulation); Fri, 24 May 2019 16:25:00 +0700 From: =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= To: git@vger.kernel.org Cc: Junio C Hamano , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= , bturner@atlassian.com, tmz@pobox.com, =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= Subject: [PATCH 3/3] parse-options: check empty value in OPT_INTEGER and OPT_ABBREV Date: Fri, 24 May 2019 16:24:42 +0700 Message-Id: <20190524092442.701-4-pclouds@gmail.com> X-Mailer: git-send-email 2.22.0.rc0.322.g2b0371e29a In-Reply-To: <20190524092442.701-1-pclouds@gmail.com> References: <20190524092442.701-1-pclouds@gmail.com> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When parsing the argument for OPT_INTEGER and OPT_ABBREV, we check if we can parse the entire argument to a number with "if (*s)". There is one missing check: if "arg" is empty to begin with, we fail to notice. This could happen with long option by writing like git diff --inter-hunk-context= blah blah Before 16ed6c97cc (diff-parseopt: convert --inter-hunk-context, 2019-03-24), --inter-hunk-context is handled by a custom parser opt_arg() and does detect this correctly. This restores the bahvior for --inter-hunk-context and make sure all other integer options are handled the same (sane) way. For OPT_ABBREV this is new behavior. But it makes it consistent with the rest. PS. OPT_MAGNITUDE has similar code but git_parse_ulong() does detect empty "arg". So it's good to go. Signed-off-by: Nguyễn Thái Ngọc Duy --- parse-options-cb.c | 3 +++ parse-options.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index 4b95d04a37..a3de795c58 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -16,6 +16,9 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) if (!arg) { v = unset ? 0 : DEFAULT_ABBREV; } else { + if (!*arg) + return error(_("option `%s' expects a numerical value"), + opt->long_name); v = strtol(arg, (char **)&arg, 10); if (*arg) return error(_("option `%s' expects a numerical value"), diff --git a/parse-options.c b/parse-options.c index 987e27cb91..87b26a1d92 100644 --- a/parse-options.c +++ b/parse-options.c @@ -195,6 +195,9 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, } if (get_arg(p, opt, flags, &arg)) return -1; + if (!*arg) + return error(_("%s expects a numerical value"), + optname(opt, flags)); *(int *)opt->value = strtol(arg, (char **)&s, 10); if (*s) return error(_("%s expects a numerical value"), From patchwork Fri May 24 09:36:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Duy Nguyen X-Patchwork-Id: 10959597 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B8AB913AD for ; Fri, 24 May 2019 09:36:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A3E802882A for ; Fri, 24 May 2019 09:36:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 95BD52886B; Fri, 24 May 2019 09:36:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5359D2882A for ; Fri, 24 May 2019 09:36:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390047AbfEXJgj (ORCPT ); Fri, 24 May 2019 05:36:39 -0400 Received: from mail-pf1-f195.google.com ([209.85.210.195]:45663 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389881AbfEXJgi (ORCPT ); Fri, 24 May 2019 05:36:38 -0400 Received: by mail-pf1-f195.google.com with SMTP id s11so4987729pfm.12 for ; Fri, 24 May 2019 02:36:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Rx+KAZewP7QhWcnC6uZ4qWDfncYVmhHvtg8fxTLiC+g=; b=evlbQOR4vJ4crfu7zY24Ol+5HINS7+9Ru0MYmb3wMtrFznhnEEhRbgnLTR9WYQ1Uf1 kncOFpUhRFTSoViEvuA5sVVO1/uy5v05Y8Twv0oiEEg7h1f6kxeTMJM9YAmQ5uJK0bSC OXB/xzjOvdeSh+nDYBWLywze7tA7j2HK3EQQNBK4yuZWwuOQ3WCDpslTKI5z/PLsHyQz +CxXe1KXYs0dbNNNC78fDv0fhFLeVuDcB++vT5ZIBC//bVDhMxjC9jDyYqtpNlAqFbMv KIsOg7vinBO+FkvtTNF+JlLnLPjZ9gcfEFHqeiEWKXCCARL6bpDn1qddILv3TK62xGS5 osWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Rx+KAZewP7QhWcnC6uZ4qWDfncYVmhHvtg8fxTLiC+g=; b=aXXZBMs0BaI8XDLACTUgxn3NmyZbBDZEfuCj48IQ05LaPPz3BByCtPZv3c0LRGJQC3 X9OM48zIabUrTu/ggDQADVrulwDxYNdjAHxpo0c9L9uqOI6Ottag3OZsFks8ckT1LkAf IlTlf0nwCbmJ+sXKkW0rZRiV4/5AG3c5CG9bwNH7IsqkaxlFeXsJTPHagraT3usYwcMG ob9dPeg2p04DVcaEy7HdSNaH+JKVqy0DHY2xpCJorw8qGFXXAibiWG8/9t18M67K5xsr 5wEXYA2jDowhWuQj35WZ8N9/+otaKZ2LEAk5sDGbCzcocoLarsKWQbZ6WlvTXakrCxUe +W8w== X-Gm-Message-State: APjAAAUDOm49ZkAf4hbka/Yy9dWeJQyJNvBvowL1IIgK6ElL4J2yjGwe MpvdXMFOoTVtnCi04vPIeOc= X-Google-Smtp-Source: APXvYqzkmKkmPoJxuovoVMUD7wIw6xaKdcYo797T7K/FvkkqgWmhsvh//99kM2uo5iFCy4KKPBS8wA== X-Received: by 2002:a65:56c3:: with SMTP id w3mr10428175pgs.232.1558690597232; Fri, 24 May 2019 02:36:37 -0700 (PDT) Received: from ash ([115.72.28.243]) by smtp.gmail.com with ESMTPSA id 85sm1923195pgb.52.2019.05.24.02.36.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 02:36:36 -0700 (PDT) Received: by ash (sSMTP sendmail emulation); Fri, 24 May 2019 16:36:32 +0700 From: =?utf-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41jIER1eQ==?= To: pclouds@gmail.com Cc: avarab@gmail.com, bturner@atlassian.com, git@vger.kernel.org, gitster@pobox.com, tmz@pobox.com Subject: [PATCH 4/3] parse-options: make compiler check value type mismatch Date: Fri, 24 May 2019 16:36:11 +0700 Message-Id: <20190524093611.1165-1-pclouds@gmail.com> X-Mailer: git-send-email 2.22.0.rc0.322.g2b0371e29a In-Reply-To: <20190524092442.701-1-pclouds@gmail.com> References: <20190524092442.701-1-pclouds@gmail.com> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP There is a disconnect between the actual value type from parse-options user and the parser itself. This is because we have to store 'value' pointer as 'void *' and the compiler cannot help point out that the user is passing a 'long' while the parser expects an 'int'. This could lead to memory corruption. In order to spot these type mismatch problems, a dummy inline function is used to process the 'value' input with the right type, before the input is stored in 'struct option'. This gives the compiler some context to start complaining. The catch though, is that we can only call a function in variable declaration if it's in automatic scope. Global and static 'struct option' variables will fail to build after this. But I think this is an reasonable price to pay, compared to memory corruption. Signed-off-by: Nguyễn Thái Ngọc Duy --- For the record, this is what I used to make patch 1/3 (I don't think I could just rely on manual code inspection to catch these problems) If we are doing something like this, then we have some clean up to do first. I think it's worth doing though. But maybe there's a better way? parse-options.h | 50 ++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/parse-options.h b/parse-options.h index ac6ba8abf9..b6ea0ac66d 100644 --- a/parse-options.h +++ b/parse-options.h @@ -128,55 +128,67 @@ struct option { intptr_t extra; }; -#define OPT_BIT_F(s, l, v, h, b, f) { OPTION_BIT, (s), (l), (v), NULL, (h), \ +#define DEFINE_OPT_TYPE_CHECK(name, type) \ + static inline void *_opt_ ## name(type *p) \ + { \ + return p; \ + } + +DEFINE_OPT_TYPE_CHECK(int, int) +DEFINE_OPT_TYPE_CHECK(ulong, unsigned long) +DEFINE_OPT_TYPE_CHECK(string, const char *) +DEFINE_OPT_TYPE_CHECK(string_list, struct string_list) +DEFINE_OPT_TYPE_CHECK(timestamp, timestamp_t) + +#define OPT_BIT_F(s, l, v, h, b, f) { OPTION_BIT, (s), (l), _opt_int(v), NULL, (h), \ PARSE_OPT_NOARG|(f), NULL, (b) } -#define OPT_COUNTUP_F(s, l, v, h, f) { OPTION_COUNTUP, (s), (l), (v), NULL, \ +#define OPT_COUNTUP_F(s, l, v, h, f) { OPTION_COUNTUP, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG|(f) } -#define OPT_SET_INT_F(s, l, v, h, i, f) { OPTION_SET_INT, (s), (l), (v), NULL, \ +#define OPT_SET_INT_F(s, l, v, h, i, f) { OPTION_SET_INT, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG | (f), NULL, (i) } #define OPT_BOOL_F(s, l, v, h, f) OPT_SET_INT_F(s, l, v, h, 1, f) #define OPT_CALLBACK_F(s, l, v, a, h, f, cb) \ { OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), (cb) } -#define OPT_STRING_F(s, l, v, a, h, f) { OPTION_STRING, (s), (l), (v), (a), (h), (f) } -#define OPT_INTEGER_F(s, l, v, h, f) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h), (f) } +#define OPT_STRING_F(s, l, v, a, h, f) { OPTION_STRING, (s), (l), _opt_string(v), (a), (h), (f) } +#define OPT_INTEGER_F(s, l, v, h, f) { OPTION_INTEGER, (s), (l), _opt_int(v), N_("n"), (h), (f) } #define OPT_END() { OPTION_END } -#define OPT_ARGUMENT(l, v, h) { OPTION_ARGUMENT, 0, (l), (v), NULL, \ +#define OPT_ARGUMENT(l, v, h) { OPTION_ARGUMENT, 0, (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG, NULL, 1 } #define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, (h) } #define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0) -#define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \ +#define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), _opt_int(v), NULL, (h), \ PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, \ (set), NULL, (clear) } -#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \ +#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG, NULL, (b) } #define OPT_COUNTUP(s, l, v, h) OPT_COUNTUP_F(s, l, v, h, 0) #define OPT_SET_INT(s, l, v, h, i) OPT_SET_INT_F(s, l, v, h, i, 0) #define OPT_BOOL(s, l, v, h) OPT_BOOL_F(s, l, v, h, 0) -#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \ +#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1} -#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \ +#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } #define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0) -#define OPT_MAGNITUDE(s, l, v, h) { OPTION_MAGNITUDE, (s), (l), (v), \ +#define OPT_MAGNITUDE(s, l, v, h) { OPTION_MAGNITUDE, (s), (l), _opt_ulong(v), \ N_("n"), (h), PARSE_OPT_NONEG } #define OPT_STRING(s, l, v, a, h) OPT_STRING_F(s, l, v, a, h, 0) #define OPT_STRING_LIST(s, l, v, a, h) \ - { OPTION_CALLBACK, (s), (l), (v), (a), \ + { OPTION_CALLBACK, (s), (l), _opt_string_list(v), (a), \ (h), 0, &parse_opt_string_list } -#define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), NULL, \ +#define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), _opt_int(v), NULL, \ (h), PARSE_OPT_NOARG, &parse_opt_tertiary } #define OPT_EXPIRY_DATE(s, l, v, h) \ - { OPTION_CALLBACK, (s), (l), (v), N_("expiry-date"),(h), 0, \ + { OPTION_CALLBACK, (s), (l), _opt_timestamp(v), N_("expiry-date"),(h), 0, \ parse_opt_expiry_date_cb } #define OPT_CALLBACK(s, l, v, a, h, f) OPT_CALLBACK_F(s, l, v, a, h, 0, f) #define OPT_NUMBER_CALLBACK(v, h, f) \ { OPTION_NUMBER, 0, NULL, (v), NULL, (h), \ PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) } -#define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \ +#define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), _opt_string(v), \ N_("file"), (h) } #define OPT_COLOR_FLAG(s, l, v, h) \ - { OPTION_CALLBACK, (s), (l), (v), N_("when"), (h), PARSE_OPT_OPTARG, \ + { OPTION_CALLBACK, (s), (l), _opt_int(v), N_("when"), (h), PARSE_OPT_OPTARG, \ parse_opt_color_flag_cb, (intptr_t)"always" } #define OPT_NOOP_NOARG(s, l) \ @@ -301,14 +313,14 @@ int parse_opt_passthru_argv(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h) OPT_COUNTUP('q', "quiet", (var), (h)) #define OPT__VERBOSITY(var) \ - { OPTION_CALLBACK, 'v', "verbose", (var), NULL, N_("be more verbose"), \ + { OPTION_CALLBACK, 'v', "verbose", _opt_int(var), NULL, N_("be more verbose"), \ PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \ - { OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \ + { OPTION_CALLBACK, 'q', "quiet", _opt_int(var), NULL, N_("be more quiet"), \ PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 } #define OPT__DRY_RUN(var, h) OPT_BOOL('n', "dry-run", (var), (h)) #define OPT__FORCE(var, h, f) OPT_COUNTUP_F('f', "force", (var), (h), (f)) #define OPT__ABBREV(var) \ - { OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \ + { OPTION_CALLBACK, 0, "abbrev", _opt_int(var), N_("n"), \ N_("use digits to display SHA-1s"), \ PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 } #define OPT__COLOR(var, h) \