[lttng-dev] [PATCH] lttng-tools conf.c and destroy.c

[Thibault, Daniel]

-----Message d'origine-----
De : David Goulet [mailto:david.goulet at polymtl.ca] 
Envoyé : 1 février 2012 12:28

> > destroy.c :
> > * Fix usage() output alignment
> > * Document and enforce return values of cmd_destroy() and destroy_session()
> > * Output --help to stdout
> > * Fix destroy_session() so it removes the config when the current session is explicitly specified
> > conf.c :
> > * Fix config_destroy() description to match behaviour

> I will not merge this before rc1. This changes the destroy command and unless
> there is a bug or important behavior fix, I'll wait after the stable release.

   You mean it is by design that the config survives if the current session is explicitly destroyed?  Please explain.

> > +	if ((cur_session_name == NULL) && (opt_session_name == NULL)) {
>
> Extra parentheses are useless here. (Just for "homogeneity" across the code base).

   True.  Is 'variable == NULL' preferred to '!variable'?

> > +	destroying_current_session =
> > +			(opt_session_name == NULL) ||
> > +			(cur_session_name ?
> > +					(strcmp(cur_session_name, opt_session_name)==0) : 0);
>
> You seem to like compact "if..else" ;). I have no problem with some... "obvious"
> case but here I'll rather prefer basic if...else for code clarity and avoid
> confusion in the future.

   Yeah, that was a tricky one.

> Finally, small detail which can save me time merging you patch. The code is
> standardize on 80 characters per line (kernel standard). Most of the time you
> are OK but comments are often unaligned. I don't know if Eclipse can allow you
> that but it will be very helpful.

   You mean when I shrink comment widths to less than 80 characters?  I do that for legibility, to avoid situations where a long line is followed by a very short one.  I guess I can eschew that in the future, even if it pains me a bit.  :-)

Daniel U. Thibault
R & D pour la défense Canada - Valcartier (RDDC Valcartier) / Defence R&D Canada - Valcartier (DRDC Valcartier)
Système de systèmes (SdS) / System of Systems (SoS)
Solutions informatiques et expérimentations (SIE) / Computing Solutions and Experimentations (CSE)
2459 Boul. Pie XI Nord
Québec, QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC: 918V QSDJ
Gouvernement du Canada / Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>