linux-kernel - Re: [PATCH v2 14/14] perf tools: Move subcommand framework and related utils to libapi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 11 Dec 2015 08:21:14 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Josh Poimboeuf <jpoimboe@...hat.com>
Cc:	Namhyung Kim <namhyung@...nel.org>, Ingo Molnar <mingo@...nel.org>,
	Jiri Olsa <jolsa@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2 14/14] perf tools: Move subcommand framework and
 related utils to libapi

Em Thu, Dec 10, 2015 at 03:35:24PM -0600, Josh Poimboeuf escreveu:
> 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).

Well, what you're doing is an improvement, so lets go with it. But I'm
not afraid of at some point doing a N+ files change if that makes sense,
and I find any panic() like libary function to be horrible and in need
of removing.
 
> So I'm thinking I'll leave the code as it is for now and just document
> the fact that these functions can exit().

Fair enough, as code using this library will live in tools/ we can
always change all the callers in one go when we see fit.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/