[PATCH 0/5] Miscellaneous pacman-conf tweaks

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/5] Miscellaneous pacman-conf tweaks

Ivy Foster-2
From: Ivy Foster <[hidden email]>

I started fiddling around with pacman-conf today, because I wanted to
add short options to it, and then just sort of...kept on for a bit.

Oh, I could be wrong, but I think that 'pacman-conf --repo <foo>'
pretty much covers [FS#25568][1].

[1]: https://bugs.archlinux.org/task/25568

Ivy Foster (5):
  pacman-conf: accept short options
  pacman-conf: simplify usage()
  pacman-conf: Make myname and myver into cpp macros MYNAME and MYVER
  pacman-conf: Make cleanup run automagically at exit
  pacman-conf.c: make parse_opts only parse opts

 src/pacman/pacman-conf.c | 84 +++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 41 deletions(-)

--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5] pacman-conf: accept short options

Ivy Foster-2
From: Ivy Foster <[hidden email]>

Also change '-?' no longer to be an error, because if we're going to
check for it anyway, why make it an error?

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/pacman-conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index 1e6f55f9..5347a837 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -59,7 +59,7 @@ static void parse_opts(int argc, char **argv)
  int c;
  config_file = CONFFILE;
 
- const char *short_opts = "";
+ const char *short_opts = "c:hlR:r:Vv?";
  struct option long_opts[] = {
  { "config"    , required_argument , NULL , 'c' },
  { "rootdir"   , required_argument , NULL , 'R' },
@@ -89,6 +89,7 @@ static void parse_opts(int argc, char **argv)
  verbose = 1;
  break;
  case 'h':
+ case '?':
  usage(0);
  break;
  case 'V':
@@ -96,7 +97,6 @@ static void parse_opts(int argc, char **argv)
  cleanup();
  exit(0);
  break;
- case '?':
  default:
  usage(1);
  break;
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/5] pacman-conf: simplify usage()

Ivy Foster-2
In reply to this post by Ivy Foster-2
From: Ivy Foster <[hidden email]>

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/pacman-conf.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index 5347a837..a503ece9 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -34,24 +34,21 @@ static void cleanup(void)
  config_free(config);
 }
 
-static void usage(int ret)
+static void usage(FILE *stream)
 {
- FILE *stream = (ret ? stderr : stdout);
-#define hputs(x) fputs(x"\n", stream)
- hputs("pacman-conf - query pacman's configuration file");
- hputs("usage:  pacman-conf [options] [<directive>...]");
- hputs("        pacman-conf (--repo-list|--help|--version)");
- hputs("options:");
- hputs("  --config=<path>  set an alternate configuration file");
- hputs("  --rootdir=<path> set an alternate installation root");
- hputs("  --repo=<remote>  query options for a specific repo");
- hputs("  --verbose        always show directive names");
- hputs("  --repo-list      list configured repositories");
- hputs("  --help           display this help information");
- hputs("  --version        display version information");
-#undef hputs
- cleanup();
- exit(ret);
+ static const char help[] =
+ "pacman-conf: query pacman's configuration file\n"
+ "usage: pacman-conf [options] [directive]\n"
+ "       pacman-conf [ { -l, --repo-list } | { -h, --help } | { -V, --version } ]\n"
+ "options:\n"
+ "    -c, --config=<file>  Read configuration from <file>\n"
+ "    -h, --help           Print help\n"
+ "    -l, --repo-list      List configured repositories\n"
+ "    -r, --repo=<remote>  Query options for a specific repo\n"
+ "    -R, --rootdir=<dir>  Use <dir> as system root\n"
+ "    -v, --verbose        Always show directive names\n"
+ "    -V, --version        Print version\n";
+ fprintf(stream, "%s", help);
 }
 
 static void parse_opts(int argc, char **argv)
@@ -90,15 +87,18 @@ static void parse_opts(int argc, char **argv)
  break;
  case 'h':
  case '?':
- usage(0);
- break;
+ usage(stdout);
+ cleanup();
+ exit(0);
  case 'V':
  printf("%s v%s\n", myname, myver);
  cleanup();
  exit(0);
  break;
  default:
- usage(1);
+ usage(stderr);
+ cleanup();
+ exit(1);
  break;
  }
  }
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5] pacman-conf: Make myname and myver into cpp macros MYNAME and MYVER

Ivy Foster-2
In reply to this post by Ivy Foster-2
From: Ivy Foster <[hidden email]>

This allows MYNAME to be used in place of the literal string
"pacman-conf" in usage

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/pacman-conf.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index a503ece9..7d09fd4b 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -21,7 +21,8 @@
 #include <string.h>
 #include "conf.h"
 
-const char *myname = "pacman-conf", *myver = "1.0.0";
+#define MYNAME "pacman-conf"
+#define MYVER "1.0.0"
 
 alpm_list_t *directives = NULL;
 char sep = '\n', *repo_name = NULL;
@@ -37,9 +38,9 @@ static void cleanup(void)
 static void usage(FILE *stream)
 {
  static const char help[] =
- "pacman-conf: query pacman's configuration file\n"
- "usage: pacman-conf [options] [directive]\n"
- "       pacman-conf [ { -l, --repo-list } | { -h, --help } | { -V, --version } ]\n"
+ MYNAME ": query pacman's configuration file\n"
+ "usage: " MYNAME " [options] [directive]\n"
+ "       " MYNAME " [ { -l, --repo-list } | { -h, --help } | { -V, --version } ]\n"
  "options:\n"
  "    -c, --config=<file>  Read configuration from <file>\n"
  "    -h, --help           Print help\n"
@@ -91,7 +92,7 @@ static void parse_opts(int argc, char **argv)
  cleanup();
  exit(0);
  case 'V':
- printf("%s v%s\n", myname, myver);
+ printf("%s v%s\n", MYNAME, MYVER);
  cleanup();
  exit(0);
  break;
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5] pacman-conf: Make cleanup run automagically at exit

Ivy Foster-2
In reply to this post by Ivy Foster-2
From: Ivy Foster <[hidden email]>

Since using atexit(3) draws in stdlib anyway and this changes every
exit call at large and return call in main(), go ahead and use
EXIT_SUCCESS and EXIt_FAILURE, too.

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/pacman-conf.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index 7d09fd4b..aa102031 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -18,7 +18,9 @@
  */
 
 #include <getopt.h>
+#include <stdlib.h>
 #include <string.h>
+
 #include "conf.h"
 
 #define MYNAME "pacman-conf"
@@ -89,18 +91,13 @@ static void parse_opts(int argc, char **argv)
  case 'h':
  case '?':
  usage(stdout);
- cleanup();
- exit(0);
+ exit(EXIT_SUCCESS);
  case 'V':
  printf("%s v%s\n", MYNAME, MYVER);
- cleanup();
- exit(0);
- break;
+ exit(EXIT_SUCCESS);
  default:
  usage(stderr);
- cleanup();
- exit(1);
- break;
+ exit(EXIT_FAILURE);
  }
  }
 
@@ -401,11 +398,15 @@ int main(int argc, char **argv)
 {
  int ret = 0;
 
+ if (atexit(cleanup) != 0) {
+ fputs("error: cannot set exit function\n", stderr);
+ return EXIT_FAILURE;
+ }
+
  config = config_new();
  parse_opts(argc, argv);
  if(!config) {
- ret = 1;
- goto cleanup;
+ return EXIT_FAILURE;
  }
 
  for(; optind < argc; optind++) {
@@ -419,8 +420,7 @@ int main(int argc, char **argv)
  if(repo_list) {
  if(directives) {
  fputs("error: directives may not be specified with --repo-list\n", stderr);
- ret = 1;
- goto cleanup;
+ return EXIT_FAILURE;
  }
  list_repos();
  } else if(repo_name) {
@@ -429,10 +429,7 @@ int main(int argc, char **argv)
  ret = list_directives();
  }
 
-cleanup:
- cleanup();
-
- return ret;
+ return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
 /* vim: set ts=2 sw=2 noet: */
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/5] pacman-conf.c: make parse_opts only parse opts

Ivy Foster-2
In reply to this post by Ivy Foster-2
From: Ivy Foster <[hidden email]>

Previously, it also parsed the config file instead of simply setting
the (global) config_file variable

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/pacman-conf.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index aa102031..52bbdb9e 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -100,10 +100,6 @@ static void parse_opts(int argc, char **argv)
  exit(EXIT_FAILURE);
  }
  }
-
- if(parseconfigfile(config_file) != 0 || setdefaults(config) != 0) {
- fprintf(stderr, "error parsing '%s'\n", config_file);
- }
 }
 
 static void list_repos(void)
@@ -403,8 +399,16 @@ int main(int argc, char **argv)
  return EXIT_FAILURE;
  }
 
- config = config_new();
  parse_opts(argc, argv);
+
+ if(!(config = config_new())) {
+ /* config_new prints the appropriate error message */
+ return EXIT_FAILURE;
+ }
+ if(parseconfigfile(config_file) != 0 || setdefaults(config) != 0) {
+ fprintf(stderr, "error parsing '%s'\n", config_file);
+ return EXIT_FAILURE;
+ }
  if(!config) {
  return EXIT_FAILURE;
  }
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/5] Miscellaneous pacman-conf tweaks

Eli Schwartz-2
In reply to this post by Ivy Foster-2
On 02/08/2018 11:49 PM, [hidden email] wrote:
> From: Ivy Foster <[hidden email]>
>
> I started fiddling around with pacman-conf today, because I wanted to
> add short options to it, and then just sort of...kept on for a bit.
>
> Oh, I could be wrong, but I think that 'pacman-conf --repo <foo>'
> pretty much covers [FS#25568][1].
>
> [1]: https://bugs.archlinux.org/task/25568

I agree, and I've closed it as implemented. :)

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] pacman-conf: accept short options

Eli Schwartz-2
In reply to this post by Ivy Foster-2
On 02/08/2018 11:49 PM, [hidden email] wrote:
> From: Ivy Foster <[hidden email]>
>
> Also change '-?' no longer to be an error, because if we're going to
> check for it anyway, why make it an error?

Hmm, why are we checking for it at all? I see no reason to treat it
differently from any other unknown option, and I *just* removed the
undocumented --usage variant from vercmp as I think [-h|--help] is more
than enough for everyone and everything.

Can we just remove '-?' altogether? We don't use it anywhere else,
--help is pretty standard, and I feel like '-?' will just be confusing
and possibly too Windows'y to boot.

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] pacman-conf: accept short options

Andrew Gregory
In reply to this post by Ivy Foster-2
On 02/08/18 at 10:49pm, [hidden email] wrote:
> From: Ivy Foster <[hidden email]>
>
> Also change '-?' no longer to be an error, because if we're going to
> check for it anyway, why make it an error?

Because '?' indicates an error.