From d72c3a6e165ee1abac3628c91f0999ffed0dcd81 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 14 Nov 2019 20:13:56 -0800 Subject: [PATCH] Move the generated version from a .h to a .c file Reinstate commit 4a438e51 that was reverted by commit 91b38069. That commit reversion was a mistake because it appeared, based on the problem description, that the change incorrectly constrained the valid version strings. Subsequent analysis of the problem showed that not to be the case and the problem reporter was, in fact, attempting to use a non-semantic version string. The SH_VERSION preprocessor symbol has bothered me for a long time. Especially as used in the context of `e_version[]`. This simplifies how the version is injected by the build process into the code; notwithstanding the large number of files touched by this change. --- config_ast.h.in | 1 + scripts/builtin_documentation.sh | 2 - src/cmd/ksh93/data/variables.c | 3 +- src/cmd/ksh93/docs/ksh_mini.1 | 2 +- src/cmd/ksh93/edit/emacs.c | 4 +- src/cmd/ksh93/edit/vi.c | 4 +- src/cmd/ksh93/include/meson.build | 6 --- src/cmd/ksh93/include/name.h | 1 - src/cmd/ksh93/include/shcmd.h | 3 ++ src/cmd/ksh93/include/shell.h | 3 +- src/cmd/ksh93/include/version.h.in | 3 -- src/cmd/ksh93/meson.build | 1 - src/cmd/ksh93/sh/init.c | 50 ++++++--------------- src/cmd/ksh93/sh/meson.build | 5 +++ src/cmd/ksh93/sh/version.c.in | 12 +++++ src/cmd/ksh93/tests/coprocess.sh | 1 - src/lib/libast/include/ast.h | 1 - src/lib/libast/string/fmtident.c | 72 ------------------------------ src/lib/libast/string/meson.build | 1 - 19 files changed, 43 insertions(+), 132 deletions(-) delete mode 100644 src/cmd/ksh93/include/meson.build delete mode 100644 src/cmd/ksh93/include/version.h.in create mode 100644 src/cmd/ksh93/sh/version.c.in delete mode 100644 src/lib/libast/string/fmtident.c diff --git a/config_ast.h.in b/config_ast.h.in index bc551a4d4ea4..615ebb4b9d19 100644 --- a/config_ast.h.in +++ b/config_ast.h.in @@ -225,6 +225,7 @@ typedef void (*sighandler_t)(int); // Types that are needed in many places but which are difficult to define in the header containing // the structure definition due to circular includes that would otherwise result. +typedef struct Namfun Namfun_t; // from name.h typedef struct Namval Namval_t; // from name.h typedef struct Shell_s Shell_t; // from shell.h typedef struct Shbltin_s Shbltin_t; // from shcmd.h diff --git a/scripts/builtin_documentation.sh b/scripts/builtin_documentation.sh index e5a1a2c83564..9d921362ed44 100755 --- a/scripts/builtin_documentation.sh +++ b/scripts/builtin_documentation.sh @@ -3,8 +3,6 @@ # the raw markup text. out_file="$MESON_BUILD_ROOT/$MESON_SUBDIR/documentation.c" exec > "$out_file" -# `ksh_mini.1` uses `SH_VERSION` macro to generate version number string -echo '#include "version.h"' for in_file in "$MESON_SOURCE_ROOT/src/cmd/ksh93/docs/"*.1 do cmd_name=$(basename "$in_file" .1) diff --git a/src/cmd/ksh93/data/variables.c b/src/cmd/ksh93/data/variables.c index 9c48966bd9bf..5d1b2658b9a8 100644 --- a/src/cmd/ksh93/data/variables.c +++ b/src/cmd/ksh93/data/variables.c @@ -26,7 +26,6 @@ #include "defs.h" #include "name.h" #include "shtable.h" -#include "variables.h" // IWYU pragma: keep // // This is the list of built-in shell variables and default values and default attributes. @@ -58,7 +57,7 @@ const struct shtable2 shtab_variables[] = { {".sh.subscript", 0, NULL}, {".sh.subshell", NV_INTEGER | NV_SHORT | NV_NOFREE, NULL}, {".sh.value", 0, NULL}, - {".sh.version", NV_NOFREE, (char *)(&e_version[10])}, + {".sh.version", NV_NOFREE, NULL}, {"CDPATH", 0, NULL}, {"COLUMNS", 0, NULL}, {"COMPREPLY", 0, NULL}, diff --git a/src/cmd/ksh93/docs/ksh_mini.1 b/src/cmd/ksh93/docs/ksh_mini.1 index 9e9b205fc87f..1e3bc2bc8881 100644 --- a/src/cmd/ksh93/docs/ksh_mini.1 +++ b/src/cmd/ksh93/docs/ksh_mini.1 @@ -1,5 +1,5 @@ +[-1? -@(#)$Id: sh (AT&T Research) " SH_RELEASE " $ +@(#)$Id: sh (AT&T Research) 2020.0.0 $ ] [+NAME?\b\f?\f\b - Shell, the standard command language interpreter] [+DESCRIPTION?\b\f?\f\b is a command language interpreter that executes commands read from a command line string, the standard input, or a specified file.] diff --git a/src/cmd/ksh93/edit/emacs.c b/src/cmd/ksh93/edit/emacs.c index aa778de94b6f..67e70426a602 100644 --- a/src/cmd/ksh93/edit/emacs.c +++ b/src/cmd/ksh93/edit/emacs.c @@ -253,7 +253,7 @@ int ed_emacsread(void *context, int fd, char *buff, int scend, int reedit) { goto do_default_processing; } case cntl('V'): { - emacs_show_info(ep, fmtident(e_version)); + emacs_show_info(ep, e_version); continue; } case '\0': { @@ -662,7 +662,7 @@ static_fn int emacs_escape(Emacs_t *ep, wchar_t *out, int count) { if (value < 0) value = 1; switch (ch = i) { case cntl('V'): { - emacs_show_info(ep, fmtident(e_version)); + emacs_show_info(ep, e_version); return -1; } case ' ': { diff --git a/src/cmd/ksh93/edit/vi.c b/src/cmd/ksh93/edit/vi.c index 09e2334406ea..3739477e8424 100644 --- a/src/cmd/ksh93/edit/vi.c +++ b/src/cmd/ksh93/edit/vi.c @@ -422,10 +422,12 @@ static_fn int cntlmode(Vi_t *vp) { break; } case cntl('V'): { - const char *p = fmtident(e_version); vi_save_v(vp); vi_del_line(vp, BAD); + + const char *p = e_version; while ((c = *p++)) append(vp, c, APPEND); + vi_refresh(vp, CONTROL); ed_getchar(vp->ed, -1); vi_restore_v(vp); diff --git a/src/cmd/ksh93/include/meson.build b/src/cmd/ksh93/include/meson.build deleted file mode 100644 index 3dcffc61572f..000000000000 --- a/src/cmd/ksh93/include/meson.build +++ /dev/null @@ -1,6 +0,0 @@ -vcs_version = vcs_tag(command: ['git', 'describe', '--always', '--dirty', '--tags'], - input: 'version.h.in', output: 'version.h', fallback: get_option('fallback-version-number')) - -# Yes, vcs_version is a header rather than a .c file but we have to define a dependency to force it -# to ge generated. -ksh93_files += [vcs_version] diff --git a/src/cmd/ksh93/include/name.h b/src/cmd/ksh93/include/name.h index 721d7e55c7e7..61ba5eca83b3 100644 --- a/src/cmd/ksh93/include/name.h +++ b/src/cmd/ksh93/include/name.h @@ -201,7 +201,6 @@ struct Value { #define IS_VT(value_obj, which) is_vt(value_obj, which) #define IS_VTP(value_objp, which) is_vtp(value_objp, which) -typedef struct Namfun Namfun_t; typedef struct Namdisc Namdisc_t; typedef struct Nambfun Nambfun_t; typedef struct Namarray Namarr_t; diff --git a/src/cmd/ksh93/include/shcmd.h b/src/cmd/ksh93/include/shcmd.h index 4cd063f6f549..31d7ebf2405b 100644 --- a/src/cmd/ksh93/include/shcmd.h +++ b/src/cmd/ksh93/include/shcmd.h @@ -25,6 +25,8 @@ #ifndef _SHCMD_H #define _SHCMD_H 1 +#include "sfio.h" + #define SH_PLUGIN_VERSION 20111111L // #define SHLIB(m) @@ -54,5 +56,6 @@ struct Shbltin_s { #define bltin_checksig(c) (c && c->sigset) extern int cmdinit(int, char **, Shbltin_t *, int); +extern Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp); #endif // _SHCMD_H diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index 5be1387ddc67..7865c936ca0f 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -87,7 +87,8 @@ extern const char e_format[]; extern const char e_number[]; extern const char e_restricted[]; extern const char e_recursive[]; -extern char e_version[]; +extern const char *e_version; +extern const char *ksh_version; typedef struct sh_scope { struct sh_scope *par_scope; diff --git a/src/cmd/ksh93/include/version.h.in b/src/cmd/ksh93/include/version.h.in deleted file mode 100644 index cd53b6cd18e1..000000000000 --- a/src/cmd/ksh93/include/version.h.in +++ /dev/null @@ -1,3 +0,0 @@ -// This file is used as input to the meson vcs_tag function to generate the real header. - -#define SH_RELEASE "@VCS_TAG@" diff --git a/src/cmd/ksh93/meson.build b/src/cmd/ksh93/meson.build index 3cd6e478b7b4..f979f80c9928 100644 --- a/src/cmd/ksh93/meson.build +++ b/src/cmd/ksh93/meson.build @@ -5,7 +5,6 @@ ksh93_incdir = include_directories('include', '../../lib/libast/include/', '../../lib/libdll/') -subdir('include') subdir('bltins') subdir('cmds') subdir('data') diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 9ffd56259a61..871ef7692cf3 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -61,25 +61,11 @@ #include "shtable.h" #include "stk.h" #include "variables.h" -#include "version.h" #if USE_SPAWN #include "spawnvex.h" #endif -char e_version[] = - "\n@(#)$Id: Version " -#define ATTRS 1 - "A" -#if SHOPT_BASH -#define ATTRS 1 - "B" -#endif -#if ATTRS - " " -#endif - SH_RELEASE " $\0\n"; - #if SHOPT_BASH extern void bash_init(Shell_t *, int); #endif @@ -837,52 +823,45 @@ static_fn char *name_match(const Namval_t *np, Namfun_t *fp) { static const Namdisc_t SH_MATCH_disc = { .dsize = sizeof(struct match), .getval = get_match, .namef = name_match}; -static_fn char *get_version(Namval_t *np, Namfun_t *fp) { return nv_getv(np, fp); } +static_fn char *get_version(Namval_t *np, Namfun_t *fp) { + UNUSED(np); + UNUSED(fp); + + return (char *)ksh_version; // discard const qualifier -- we expect the caller to not mutate +} // This is invoked when var `.sh.version` is used in a numeric context such as // `$(( .sh.version ))`. -static_fn Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) { +Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) { UNUSED(np); + UNUSED(fp); // We should not need to convert version number string every time this function is called static Sflong_t version_number = -1; if (version_number != -1) return (Sfdouble_t)version_number; - char *cp = strdup(SH_RELEASE); - char *dash; char *dot; - char *major_str, *minor_str, *patch_str; + const char *major_str, *minor_str, *patch_str; int major, minor, patch; - UNUSED(fp); - - // Version string in development version could be set like 2017.0.0-devel-1509-g95d59865 - // If a '-' exists in version string, set it as end of string i.e. version string becomes - // 2017.0.0 - dash = strchr(cp, '-'); - if (dash) *dash = 0; // Major version number starts at beginning of string - major_str = cp; + major_str = ksh_version; // Find the first '.' and set it to NULL, so major version string is set to 2017 - dot = strchr(cp, '.'); + dot = strchr(ksh_version, '.'); if (!dot) { // If there is no . in version string, it means version string is either empty, invalid // or it's using old versioning scheme. - free(cp); version_number = 0; return version_number; } - *dot = 0; - // Minor version string starts after first '.' minor_str = dot + 1; // Find the second '.' and set it to NULL, so minor version string is set to 0 dot = strchr(minor_str, '.'); assert(dot); - *dot = 0; // Patch number starts after second '.' patch_str = dot + 1; @@ -893,10 +872,9 @@ static_fn Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) { assert(minor < 100); assert(patch < 100); - // This will break if minor or patch number goes above 99 + // This will break if minor or patch number goes above 99. Hence the preceding assert()'s. version_number = major * 10000 + minor * 100 + patch; - free(cp); return (Sfdouble_t)version_number; } @@ -1092,8 +1070,6 @@ Shell_t *sh_init(int argc, char *argv[], Shinit_f userinit) { int type; static char *login_files[2]; - n = strlen(e_version); - if (e_version[n - 1] == '$' && e_version[n - 2] == ' ') e_version[n - 2] = 0; if (!beenhere) { beenhere = 1; shp = sh_getinterp(); @@ -1273,7 +1249,7 @@ Shell_t *sh_init(int argc, char *argv[], Shinit_f userinit) { sh_offstate(shp, SH_INIT); login_files[0] = (char *)e_profile; shp->gd->login_files = login_files; - shp->bltindata.version = SH_VERSION; + shp->bltindata.version = nget_version(NULL, NULL); shp->bltindata.shp = shp; shp->bltindata.shrun = sh_run; shp->bltindata.shexit = sh_exit; diff --git a/src/cmd/ksh93/sh/meson.build b/src/cmd/ksh93/sh/meson.build index d5841fe5c001..5478118a933a 100644 --- a/src/cmd/ksh93/sh/meson.build +++ b/src/cmd/ksh93/sh/meson.build @@ -1,4 +1,9 @@ +vcs_version = vcs_tag(command: ['git', 'describe', '--always', '--dirty', '--tags'], + input: 'version.c.in', output: 'version.c', + fallback: get_option('fallback-version-number')) + ksh93_files += [ + vcs_version, 'sh/args.c', 'sh/arith.c', 'sh/array.c', diff --git a/src/cmd/ksh93/sh/version.c.in b/src/cmd/ksh93/sh/version.c.in new file mode 100644 index 000000000000..dbb32d32dc51 --- /dev/null +++ b/src/cmd/ksh93/sh/version.c.in @@ -0,0 +1,12 @@ +// This is used as input to the Meson vcs_tag() function to produce the corresponding version.c +// module. +#include "config_ast.h" // IWYU pragma: keep + +const char *ksh_version = "@VCS_TAG@"; +const char *e_version = + "Version " + "A" +#if SHOPT_BASH + "B" +#endif + " @VCS_TAG@"; diff --git a/src/cmd/ksh93/tests/coprocess.sh b/src/cmd/ksh93/tests/coprocess.sh index 00bb1ea4a088..f8af6ffdb38a 100644 --- a/src/cmd/ksh93/tests/coprocess.sh +++ b/src/cmd/ksh93/tests/coprocess.sh @@ -263,7 +263,6 @@ done exp=ksh got=$(print -r $'#00315 COATTRIBUTES=\'label=make \' -# @(#)$Id: libcoshell (AT&T Research) 2008-04-28 $ _COSHELL_msgfd=5 { { (eval \'function fun { trap \":\" 0; return 1; }; trap \"exit 0\" 0; fun; exit 1\') && PATH= print -u$_COSHELL_msgfd ksh; } || { times && echo bsh >&$_COSHELL_msgfd; } || { echo osh >&$_COSHELL_msgfd; }; } >/dev/null 2>&1' | $SHELL 5>&1) [[ $got == $exp ]] || log_error "coshell(3) identification sequence failed -- expected '$exp', got '$got'" diff --git a/src/lib/libast/include/ast.h b/src/lib/libast/include/ast.h index 29476117f33f..2a87252c331d 100644 --- a/src/lib/libast/include/ast.h +++ b/src/lib/libast/include/ast.h @@ -152,7 +152,6 @@ extern char *fgetcwd(int, char *, size_t); extern char *fmtbase(int64_t, int, int); extern char *fmtbuf(size_t); // to be used only by the "string" subsystem extern char *fmtelapsed(unsigned long, int); -extern char *fmtident(const char *); extern char *fmtfmt(const char *); extern char *fmtint(int64_t, int); extern char *fmtmatch(const char *); diff --git a/src/lib/libast/string/fmtident.c b/src/lib/libast/string/fmtident.c deleted file mode 100644 index ea8e19319ad1..000000000000 --- a/src/lib/libast/string/fmtident.c +++ /dev/null @@ -1,72 +0,0 @@ -/*********************************************************************** - * * - * This software is part of the ast package * - * Copyright (c) 1985-2011 AT&T Intellectual Property * - * and is licensed under the * - * Eclipse Public License, Version 1.0 * - * by AT&T Intellectual Property * - * * - * A copy of the License is available at * - * http://www.eclipse.org/org/documents/epl-v10.html * - * (with md5 checksum b35adb5213ca9657e911e9befb180842) * - * * - * Information and Software Systems Research * - * AT&T Research * - * Florham Park NJ * - * * - * Glenn Fowler * - * David Korn * - * Phong Vo * - * * - ***********************************************************************/ -#include "config_ast.h" // IWYU pragma: keep - -#include -#include - -#include "ast.h" - -#define IDENT 01 -#define USAGE 02 - -/* - * format what(1) and/or ident(1) string a - */ - -char *fmtident(const char *a) { - char *s = (char *)a; - char *t; - char *buf; - int i; - - i = 0; - for (;;) { - while (isspace(*s)) s++; - if (s[0] == '[') { - while (*++s && *s != '\n') { - ; - } - i |= USAGE; - } else if (s[0] == '@' && s[1] == '(' && s[2] == '#' && s[3] == ')') { - s += 4; - } else if (s[0] == '$' && s[1] == 'I' && s[2] == 'd' && s[3] == ':' && isspace(s[4])) { - s += 5; - i |= IDENT; - } else { - break; - } - } - if (i) { - i &= IDENT; - for (t = s; isprint(*t) && *t != '\n'; t++) { - if (i && t[0] == ' ' && t[1] == '$') break; - } - while (t > s && isspace(t[-1])) t--; - i = t - s; - buf = fmtbuf(i + 1); - memcpy(buf, s, i); - s = buf; - s[i] = 0; - } - return s; -} diff --git a/src/lib/libast/string/meson.build b/src/lib/libast/string/meson.build index 6111b35d9a9c..ebfa2c76baac 100644 --- a/src/lib/libast/string/meson.build +++ b/src/lib/libast/string/meson.build @@ -6,7 +6,6 @@ libast_files += [ 'string/fmtelapsed.c', 'string/fmtesc.c', 'string/fmtfmt.c', - 'string/fmtident.c', 'string/fmtint.c', 'string/fmtmatch.c', 'string/fmtmode.c',