Re: [PATCH v2 3/3] xl: new "loglvl" command

["Jan Beulich" <JBeulich@suse.com>]

>>> On 07.03.16 at 19:07, <dario.faggioli@citrix.com> wrote:
> On Mon, 2016-03-07 at 04:46 -0700, Jan Beulich wrote:
>> > > > On 04.03.16 at 19:45, <dario.faggioli@citrix.com> wrote:
>> > On Fri, 2016-03-04 at 09:48 -0700, Jan Beulich wrote:
> 
>> > > --- a/tools/libxl/libxl.c
>> > > +++ b/tools/libxl/libxl.c
>> > > @@ -5958,6 +5958,26 @@ int libxl_send_debug_keys(libxl_ctx *ctx
>> > >      return 0;
>> > >  }
>> > >  
>> > > +int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,
>> > > +                    int *lower_thresh, int *upper_thresh)
>> > > +{
>> > > +    int ret;
>> > > 
>> > As per libxl coding style, this wants to be 'r'.
>> This and everything else below look to be valid comments, but
>> it's rather frustrating that simply cloning an existing function (I
>> user the debug key ones as basis) doesn't give me valid code,
>> the more that I did scroll up and down a few pages to see
>> whether I just happened to pick a particularly bad example.
>>
> Hehe, but do you understand that, saying this, you're making it very
> likely that people will ask *you* to fix libxl_send_debug_keys() --and
> perhaps more tool side code? :-P :-P

Except that it's not just that function - as said, I did scroll up and
down, without finding (style wise) better examples. And no, I'm
not going to put together patches to deal with style issues in the
tools.

> No, jokes apart, I agree that inconsistency is a real bad thing... but
> it's an hard fight, and we do have examples spread all around the
> source code (both Xen and tools), AFAICT.

Right, and asking people myself to not follow bad examples when
adding new code, I did take all of your input to adjust the patch.
Just that in this case the set of bad examples is so large that in a
similar case in the hypervisor I probably wouldn't have dared to
ask for a style correction.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel