Messages in this thread | | | Date | Mon, 5 May 2014 10:29:48 +0200 | From | Levente Kurusa <> | Subject | Re: [PATCH v2 2/2] staging/rtl8192e: userspace ptr deref + incorrect declarations |
| |
Hi,
On Mon, May 05, 2014 at 01:59:49AM +0200, Dominique van den Broeck wrote: > > Good evening, > > Forgive my mistakes, I sent only a few patches yet and I'm still learning. > Nevertheless:
Not a problem at all, since that is what the challenge is for.
> > > 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...
Oh, I see now! Well I guess it's better to have a commit message that says WHY the change is good and WHAT did it change, not just a list of what you did.
i.e. in this case you could also inline the sparse warning or at least a part of it.
> > > 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.
That is good as well, I just prefer the latter one, but feel free to use whichever you feel better. :-)
> > > 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). >
In that case, that's good.
> > 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. >
I have this: ----------%<----------- function! GetGitCommit(commit) exec ":.!git log --oneline --pretty=\"format:\\%h ('\\%s')\" ".a:commit." -1" endfunction ---------->%-----------
> > 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.
No, if that was applied it's unneccessary to resend the full set.
> > > 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.
No, the tree's name is not needed. In fact, I should have checked it in linux-next, but I only checked Linus', and staging-next thinking, since you said it was applied, it was applied to staging-next. :-) > > > Could you please as well remove that empty line in the declarations? > > I'll do. > Cheers.
Regards, Levente Kurusa [unhandled content-type:application/pgp-signature] | |