Subject: Re: bug#76322: Make ctags a thin wrapper around etags

From: Eli Zaretskii <[email-masked]>
Date: Thu, 20 Feb 2025 11:23:12 +0200

> Date: Mon, 17 Feb 2025 00:16:58 -0800
> Cc: [email-masked]
> From: Paul Eggert <[email-masked]>
> 
> On 2025-02-16 01:31, Eli Zaretskii wrote:
> >> Date: Sun, 16 Feb 2025 00:26:02 -0800
> >> From: Paul Eggert <[email-masked]>
> >> How about if we compile a little .exe file that merely execs etags with
> >> the --ctags option? That would be the equivalent of the script.
> > 
> > That'd be fine, thanks.  But we will need to invoke etags via
> > w32_execvp because of the problems explained in emacsclient.c where
> > that function is defined (which will need to be moved to ntlib.c).
> 
> I took at shot at doing something along those lines; see revised patch 
> (attached).
> 
> Since I don't use MS-Windows or compile or test anything on that 
> platform, I quailed a bit at doing nontrivial surgery on ntlib.c. Also, 
> emacsclient.c's w32_execvp has some alternate-editor code specific to 
> emacsclient, and it uses alloca in a potentially-unsafe way, and it 
> mishandles strings longer than INT_MAX bytes, and it appears to 
> mishandle '\t', '\\' and '"' if Microsoft's documentation is to be 
> believed. So the attached patch defines its own w32_execvp_arg_repr 
> function that attempts to be standalone and free of these issues. If 
> needed later, someone with MS-Windows expertise could merge this into 
> ntlib.c and combine it with what's in emacsclient.c.

Thanks, a few minor comments below.

> +INSTALLABLE_EXES = etags${EXEEXT} $(if $(NTDIR), ctags$(EXEEXT)) \
> +  emacsclient${EXEEXT} $(CLIENTW) ebrowse${EXEEXT}
> +INSTALLABLE_SCRIPTS = $(if $(NTDIR), , ctags)

The dependence on NTDIR is sub-optimal, IMO.  For starters, it means
that one cannot reliably invoke "make" in lib-src, because (AFAIU)
NTDIR is defined in the top-level Makefile.  It is better to depend on
the value of NTLIB, which is defined in lib-src/Makefile.

Or maybe we should just depend on the value of EXEEXT?  I think that
might be preferable, because it will also work for the MS-DOS build of
Emacs, where we have the same problem with invoking Unix shell
scripts.

> +/* Return the number of bytes used to represent the string S as an
> +   MS-Windows _execvp argument, including the trailing null byte.
> +   If D is nonnull, also store the representation into D.
> +   Due to Microsoft C runtime constraints, S cannot contain '\n',
> +   and if S contains '"' the representation cannot be used for argument 0.  */
> +static size_t
> +w32_execvp_arg_repr (char const *restrict s, char *restrict d)
> +{
> +  /* See <https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments>.

This part should be under #ifdef WINDOWSNT, since it's specific to
Windows.  The MS-DOS build doesn't have these problems with execvp.

> +int
> +main (int argc, char **argv)
> +{
> +  char const *progname = argv[0];
> +  static char const option_prefix[] = "--ctags=";
> +
> +  /* Number of bytes needed to represent all etags args.  */
> +  size_t nargbytes = sizeof option_prefix - 1;
> +  for (int i = 0; i < argc; i++)
> +    nargbytes += w32_execvp_arg_repr (argv[i], NULL);
> +
> +  char const **etags_argv = malloc ((argc + 2) * sizeof *argv + nargbytes);
> +  if (etags_argv)
> +    {
> +      char *d = (char *) (etags_argv + argc + 2);
> +      etags_argv[0] = "etags";
> +      etags_argv[1] = memcpy (d, option_prefix, sizeof option_prefix - 1);
> +      d += sizeof option_prefix - 1;
> +      d += w32_execvp_arg_repr (progname, d);
> +
> +      for (int i = 1; i < argc; i++)
> +	{
> +	  etags_argv[i + 1] = d;
> +	  d += w32_execvp_arg_repr (argv[i], d);
> +	}

Likewise here: the calls to w32_execvp_arg_repr should only be done on
WINDOWSNT.

Also, this:

> +      etags_argv[0] = "etags";

basically means the etags to be invoked is the one found on PATH,
whereas I think we want to invoke etags from the same place from where
ctags was invoked.  So I would suggest keeping any leading directories
in argv[0] intact, and just replacing "ctags" with "etags" to generate
the new argv[0].

Caveat: I didn't try to actually build this stuff on Windows, let
alone run the test suite; that will need to wait till later, maybe
even till after you install this on master.

Thanks again.