lkml.org 
[lkml]   [2013]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] panic: setup panic_timeout early
From
On Fri, Nov 15, 2013 at 7:12 AM, Levente Kurusa <levex@linux.com> wrote:

> No, you don't want timeout to be a long, and instead of kstrtol, you should use
> kstrtoint(char *s, int base, int *res)
> which is defined in lib/ktstrtox.c

And if you look into kstrtoint() you would see that it's doing
*exactly* the same I'm doing; using a temporary bigger integer, same
as STANDARD_PARAM_DEF.

> Also, a bit of advice: Calm down. If you continue to act childish nobody will take
> you seriously. Before acting like you are the king here, please at least take the time
> to research other options.

I am irrelevant, the code is what matters, and panic_timeout should be
set early on, regardless of who is sending the patch.

Moreover, if you are going to claim that I didn't research other
options, then the guy that did this:

STANDARD_PARAM_DEF(int, int, "%i", long, kstrtol);

Didn't research other options either, because it should be:

STANDARD_PARAM_DEF(int, int, "%i", int, kstrtoint);

Presumably it doesn't matter much, but it seems it does matter when
*I* am the one sending the patch.

And what exactly is childish about asking this question?

> So you would rather have this?
>
> kstrtol(val, 0, (long *)&timeout);

The answer should be: no, use kstrtoint(), but that wasn't your
answer, was it? Your answer basically repeated what Ingo said, so I
had to go step by step to the conclusion of 'kstrtol(val, 0, (long
*)&timeout);'. And then I asked you a question: "What else do you
suggest to fix the problem that kstrtol() expects a long?" *Now* your
answer is useful. Why is that childish?

Now, in this process a few things are clear to me:

1) loglevel should use kstrtoint() as well, not get_option
2) get_option() should use kstrtoint(); it's using simple_strtol() and
it's not even checking for overflows
3) It should be STANDARD_PARAM_DEF(int, int, "%i", int, kstrtoint);

Don't you think it's little bit of a stretch to say that *I* didn't do
my research, when in fact I did, and none of the parameters code is
using kstrtoint() as it should, even Ingo himself told me I should use
simple_strtol(), which is actually deprecated.

My patch is simply doing what similar code is already doing. Why don't
you take a step back and accept the possibility that a) I did do my
research, and b) I wasn't being childish in asking how to make kstrtol
work (given that Ingo suggested to use simple_strtol()). I think you
should take your own advice and calm down, maybe even take back what
you said. Maybe I was combative, and maybe I made the wrong
assumptions, but at least I didn't throw insults, nor called anybody
names like "childish".

Anyway, I will update the patch to sue kstrtoint(), and I would gladly
fix the existing code for issues 1) 2) and 3) *if* you or anybody
agrees they should be using kstrtoint() as well, I'm not in the mood
of going through Ingo's review process again for something so trivial.

Cheers.

--
Felipe Contreras


\
 
 \ /
  Last update: 2013-11-15 21:21    [W:1.876 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site