From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: "#define precompose_argv(c,v) /* empty */" is evil
Date: Thu, 06 Aug 2020 16:47:34 -0700 [thread overview]
Message-ID: <xmqqy2mribft.fsf@gitster.c.googlers.com> (raw)
I had seen an interesting compilation breakage today. A topic adds
many more uses of argv-array API so I resolved semantic conflict
patch until "make builtins/submodule-helper.o" passed. I failed to
spot one remaining breakage until I saw
https://travis-ci.org/github/git/git/jobs/715668996
The problematic line was
precompose_argv(diff_args.argc, diff_args.argv);
where diff_args used to be an argv_array and is now a strvec.
Why didn't I catch this in my local test?
$ git grep -n -e precompose_argv -- \*.h
compat/precompose_utf8.h:31:void precompose_argv(int argc, const char **argv);
git-compat-util.h:256:#define precompose_argv(c,v)
The problematic part is this one used on all platforms other than macOS:
/* used on Mac OS X */
#ifdef PRECOMPOSE_UNICODE
#include "compat/precompose_utf8.h"
#else
#define precompose_str(in,i_nfd2nfc)
#define precompose_argv(c,v)
#define probe_utf8_pathname_composition()
#endif
I am wondering if it is a good idea to use something like
static inline void precompose_argv(int argc, const char **argv)
{
; /* nothing */
}
instead. As long as the compiler is reasonable enough, this should
not result in any code change in the result, except that it would
still catch wrong arguments, even if these two parameters are unused
and optimized out.
next reply other threads:[~2020-08-06 23:47 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-06 23:47 Junio C Hamano [this message]
2020-08-07 0:01 ` "#define precompose_argv(c,v) /* empty */" is evil brian m. carlson
2020-08-07 0:23 ` Junio C Hamano
2020-08-07 1:32 ` brian m. carlson
2020-08-07 4:03 ` Junio C Hamano
2020-08-07 3:27 ` Jeff King
2020-08-07 4:09 ` Junio C Hamano
2020-08-07 4:34 ` Jeff King
2020-08-07 6:42 ` Junio C Hamano
2020-08-07 7:56 ` Torsten Bögershausen
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy2mribft.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.