Re: Bash ini builtin implementation feedback

[Jesse Hathaway]

Thanks folks for the help, if anyone is interested in reading the full post
I just posted it on lobste.rs:

    https://lobste.rs/s/6wjuk4/writing_bash_builtin_c_parse_ini_configs

On Thu, Jul 1, 2021 at 9:15 AM Jesse Hathaway <jesse@mbuki-mvuki.org> wrote:
>
> I thought it would be fun to write a blog post on Bash builtins. I have never
> written a builtin before, but I thought one that parses ini config files would
> provide a good example. I have a working implementation using the inih
> library[1]. I would love any feedback on the code as I have written very 
> little
> C during my career.
>
> The full source code is below, the Makefile can be found in its repo,
> https://github.com/lollipopman/bash-ini-builtin-blog-post.
>
> Yours kindly, Jesse Hathaway
>
> [1]: https://github.com/benhoyt/inih
>
> #include "inih/ini.h"
> #include "loadables.h"
> #include <errno.h>
>
> char *ini_doc[] = {
>     "Read an ini config from stdin input into a set of associative arrays.",
>     "",
>     "Reads an ini config from stdin input into a set of associative arrays.",
>     "The sections of the ini config are added to an associative array",
>     "specified by the `-a TOC` argument. The keys and values are then added 
> to",
>     "associate arrays prefixed by the `TOC` name and suffixed by their ini",
>     "section name, `<TOC>_<INI_SECTION_NAME>`.",
>     "",
>     "Example:",
>     "",
>     "  Input input.ini:",
>     "    [sec1]",
>     "    foo = bar",
>     "",
>     "    [sec2]",
>     "    biz = baz",
>     "",
>     "  Result:",
>     "    $ ini -a conf < input.ini",
>     "    $ declare -p conf",
>     "    declare -A conf=([sec1]=\"true\" [sec2]=\"true\" )",
>     "    $ declare -p conf_sec1",
>     "    declare -A conf_sec1=([foo]=\"bar\" )",
>     "    $ declare -p conf_sec2",
>     "    declare -A conf_sec2=([biz]=\"baz\" )",
>     "",
>     "If the `-u FD` argument is passed the ini config is read from the `FD`",
>     "file descriptor rather than from stdin.",
>     (char *)NULL};
>
> /* User data for inih callback handler */
> typedef struct {
>   char *toc_var_name;
> } ini_conf;
>
> /* This is the inih handler called for every new section and for every name 
> and
>  * value in a section. This function creates and populates our associative
>  * arrays in Bash. Both for the TOC array as well as for the individual 
> section
>  * arrays, <TOC>_<INI_SECTION_NAME> */
> static int handler(void *user, const char *section, const char *name,
>                    const char *value) {
>   ini_conf *conf = (ini_conf *)user;
>   char *toc_var_name = conf->toc_var_name;
>   int vflags;
>   if (!name && !value) {
>     vflags = 0;
>     vflags |= (1 << 0);
>     vflags |= (1 << 1);
>     SHELL_VAR *toc_var = find_or_make_array_variable(toc_var_name, vflags);
>     if (!toc_var) {
>       builtin_error("Could not make %s", toc_var_name);
>       return 0;
>     }
>     bind_assoc_variable(toc_var, toc_var_name, strdup(section), "true", 0);
>     return 1;
>   }
>   if (!name) {
>     builtin_error("Malformed ini, name is NULL!");
>     return 0;
>   }
>   if (!value) {
>     builtin_error("Malformed ini, value is NULL!");
>     return 0;
>   }
>   /* Create <TOC>_<INI_SECTION_NAME> */
>   char *sep = "_";
>   size_t sec_size = strlen(toc_var_name) + strlen(section) + strlen(sep) +
>                     1; // +1 for the null-terminator
>   char *sec_var_name = malloc(sec_size);
>   char *sec_end = sec_var_name + sec_size - 1;
>   char *p = memccpy(sec_var_name, toc_var_name, '\0', sec_size);
>   if (!p) {
>     builtin_error("Unable to create section name");
>     return 0;
>   }
>   p = memccpy(p - 1, sep, '\0', sec_end - p + 2);
>   if (!p) {
>     builtin_error("Unable to create section name");
>     return 0;
>   }
>   p = memccpy(p - 1, section, '\0', sec_end - p + 2);
>   if (!p) {
>     builtin_error("Unable to create section name");
>     return 0;
>   }
>   if (!legal_identifier(sec_var_name)) {
>     sh_invalidid(sec_var_name);
>     free(sec_var_name);
>     return 0;
>   }
>   vflags = 0;
>   vflags |= (1 << 0);
>   vflags |= (1 << 1);
>   SHELL_VAR *sec_var = find_or_make_array_variable(sec_var_name, vflags);
>   if (!sec_var) {
>     builtin_error("Could not make %s", sec_var_name);
>     free(sec_var_name);
>     return 0;
>   }
>   bind_assoc_variable(sec_var, sec_var_name, strdup(name), strdup(value), 0);
>   free(sec_var_name);
>   return 1;
> }
>
> /* This is essentially the main function for the ini builtin, it does arg
>  * parsing and then calls the inih function to parse the provided ini FD */
> int ini_builtin(list) WORD_LIST *list;
> {
>   intmax_t intval;
>   int opt, code;
>   int fd = 0;
>   char *toc_var_name = NULL;
>   reset_internal_getopt();
>   while ((opt = internal_getopt(list, "a:u:")) != -1) {
>     switch (opt) {
>     case 'a':
>       toc_var_name = list_optarg;
>       break;
>     case 'u':
>       code = legal_number(list_optarg, &intval);
>       if (code == 0 || intval < 0 || intval != (int)intval) {
>         builtin_error("%s: invalid file descriptor specification", 
> list_optarg);
>         return (EXECUTION_FAILURE);
>       }
>       fd = (int)intval;
>       if (sh_validfd(fd) == 0) {
>         builtin_error("%d: invalid file descriptor: %s", fd, strerror(errno));
>         return (EXECUTION_FAILURE);
>       }
>       break;
>     case GETOPT_HELP:
>       builtin_help();
>       return (EX_USAGE);
>     default:
>       builtin_usage();
>       return (EX_USAGE);
>     }
>   }
>   if (!toc_var_name) {
>     builtin_usage();
>     return (EX_USAGE);
>   }
>   ini_conf conf = {};
>   conf.toc_var_name = strdup(toc_var_name);
>   FILE *file = fdopen(fd, "r");
>   if (ini_parse_file(file, handler, &conf) < 0) {
>     builtin_error("Unable to read from fd: %d", fd);
>     return (EXECUTION_FAILURE);
>   }
>   return (EXECUTION_SUCCESS);
> }
>
> /* Provides Bash with information about the builtin */
> struct builtin ini_struct = {
>     "ini",                /* Builtin name */
>     ini_builtin,          /* Function implementing the builtin */
>     BUILTIN_ENABLED,      /* Initial flags for builtin */
>     ini_doc,              /* Array of long documentation strings. */
>     "ini -a TOC [-u FD]", /* Usage synopsis; becomes short_doc */
>     0                     /* Reserved for internal use */
> };