linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org, Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH v2 14/14] perf tools: Move subcommand framework and related utils to libapi
Date: Thu, 10 Dec 2015 15:35:24 -0600	[thread overview]
Message-ID: <20151210213524.GC4934@treble.redhat.com> (raw)
In-Reply-To: <20151210145445.GC29872@treble.redhat.com>

On Thu, Dec 10, 2015 at 08:54:45AM -0600, Josh Poimboeuf wrote:
> On Thu, Dec 10, 2015 at 10:40:39AM +0900, Namhyung Kim wrote:
> > > - usage.c: used in several places for die() and error(), but these are
> > >   trivial functions which can be duplicated.
> > 
> > Not sure it's ok to call die() or similar in a library.  The error
> > should be reported to the caller rather than exiting inside unless
> > explicitly requested like in usage_with_options() IMHO.
> 
> Thanks, good point.  I'll try to remove all exits from the library
> (except for the explicit requests).

As it turns out, some special options like '--list-opts' and
'--list-cmds' are implemented within parse_options_subcommand(), which
then does an exit().  If those exit()'s were replaced with negative
return codes, we'd have to provide a way for callers to distinguish
between a normal early exit and a real error (in which the usage
printout might be appropriate).  That would be a disruptive change and
require the 40+ callers of the parse_options*() functions to have more
complexity (because they'd need to check for more return conditions).

So I'm thinking I'll leave the code as it is for now and just document
the fact that these functions can exit().

-- 
Josh

  reply	other threads:[~2015-12-10 21:35 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-08  4:21 [PATCH v2 00/14] perf tools: Move perf subcommand framework into lib/tools Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 01/14] perf: Fix 'make clean' Josh Poimboeuf
2015-12-08 17:40   ` Jiri Olsa
2015-12-08 18:40     ` Jiri Olsa
2015-12-08 18:49       ` Josh Poimboeuf
2015-12-08 18:46     ` Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 02/14] perf: Use -iquote for local include paths Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 03/14] perf: Split up util.h Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 04/14] perf: Move term functions out of util.c Josh Poimboeuf
2015-12-09 15:53   ` Arnaldo Carvalho de Melo
2015-12-10  8:18   ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 05/14] perf: Remove unused pager_use_color variable Josh Poimboeuf
2015-12-09 15:43   ` Arnaldo Carvalho de Melo
2015-12-10  8:18   ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 06/14] perf: Split up cache.h Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 07/14] perf: Remove cache.h Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 08/14] perf: Save cmdline arguments earlier Josh Poimboeuf
2015-12-10  8:18   ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 09/14] perf: Remove check for unused PERF_PAGER_IN_USE Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 10/14] perf: Move cmd_version() to builtin-version.c Josh Poimboeuf
2015-12-10  8:19   ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 11/14] perf: Move help_unknown_cmd() to its own file Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 12/14] perf tools: Move strlcpy() to tools/lib/string.c Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 13/14] perf tools: Move tools/lib/string.c to libapi Josh Poimboeuf
2015-12-08  4:21 ` [PATCH v2 14/14] perf tools: Move subcommand framework and related utils " Josh Poimboeuf
2015-12-08 18:16   ` Jiri Olsa
2015-12-08 18:49     ` Josh Poimboeuf
2015-12-08 19:09       ` Arnaldo Carvalho de Melo
2015-12-08 19:17         ` Josh Poimboeuf
2015-12-08 19:40           ` Arnaldo Carvalho de Melo
2015-12-08 21:48             ` Josh Poimboeuf
2015-12-08 22:27               ` Arnaldo Carvalho de Melo
2015-12-08 23:07                 ` Josh Poimboeuf
2015-12-09  8:03                   ` Ingo Molnar
2015-12-09 12:33                     ` Josh Poimboeuf
2015-12-09 15:58                       ` Arnaldo Carvalho de Melo
2015-12-09 18:59                         ` Josh Poimboeuf
2015-12-10  1:40                           ` Namhyung Kim
2015-12-10 14:54                             ` Josh Poimboeuf
2015-12-10 21:35                               ` Josh Poimboeuf [this message]
2015-12-11 11:21                                 ` Arnaldo Carvalho de Melo
2015-12-10 12:55                           ` Arnaldo Carvalho de Melo
2015-12-10 15:15                             ` Josh Poimboeuf
2015-12-10  1:58   ` Namhyung Kim
2015-12-10  2:00 ` [PATCH v2 00/14] perf tools: Move perf subcommand framework into lib/tools Namhyung Kim
2015-12-10 15:11   ` Josh Poimboeuf

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=20151210213524.GC4934@treble.redhat.com \
    --to=jpoimboe@redhat.com \
    --cc=acme@kernel.org \
    --cc=bp@alien8.de \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).