Re: [PATCH v2 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations

From: Dominique van den Broeck
Date: Sun May 04 2014 - 20:00:10 EST



Good evening,

Forgive my mistakes, I sent only a few patches yet and I'm still learning.
Nevertheless:

> What is that period in the commit message? And the semicolon?

Semicolons is what one use to ponctuate an enumerated list (at least
in french). In fact, it was their primary use before computer era.

Is there something wrong with it ? I check all my patches with
checkpatch.pl --strict before sending them and it didn't seem to
complain...

> You should also be a bit more specific. Also, the Subject line is
> very bad. Better go with something like this:
>
> staging: rtl8192e: fix userspace pointer dereference

Right. I used the slash as a subpart of the tree. Didn't know what
was the best for this situation.

> And when you resend a patchset, please resend the full patchset.

I usually do but I've got an acceptation message for the first one
(see below).

> This is totally unneccessary.

Should at least have gone in the body below indeed.

> When you cite a commit please don't include the full hash, that is
> non informational. Better put the first 7 characters of the hash and
> the first line of the commit message as well in parantheses, like so:
>
> 5169af2 ("Staging: rtl8192e: Fix declaration of symbols")
> (I even have a command for this in vim :-) )

I'm interested. I use vim too.

> Are you sure that 1/2 was applied to the staging tree?
> It's unlikely that 1/2 is applied while 2/2 is left alone.

As stated before, I received the common automatic mail from
Greg KH regarding this one. So I'll now wait before I do a v3
for this issue. If so, I'll resent the complete set if required.

> Oh, I am unable to find commit b5c8d48 in Linus' or staging-next.
> In which tree is it?

It's linux-next. If I quote a commit, I should the tree as well,
indeed. But since the v1 was performed partially for the Eudyptula
Project and since it was a response to a modification request,
I though it was implicit.

> Could you please as well remove that empty line in the declarations?

I'll do.
Cheers.



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