Re: [PATCH v2] staging: unisys: uislib: uisqueue.c: rewrite of do_locked_client_insert

Sudip Mukherjee
2014-09-03T14:43:44Z

On Wed, Sep 03, 2014 at 02:29:44PM +0530, Sudip Mukherjee wrote:
> On Wed, Sep 03, 2014 at 11:40:38AM +0300, Dan Carpenter wrote:
> > On Tue, Sep 02, 2014 at 11:46:35PM +0530, Sudip Mukherjee wrote:
> > > From: Sudip Mukherjee <sudip@vectorindia.org>
> >
> > I really would prefer if you just figured out your email settings so
> > this isn't needed. The From: header is mostly used for people
> > forwarding patches from other people. We have allowed people to use
> > the From header like this if they can't get their corporate email
> > configured properly but I try to discorage it. If everyone starts using
> > From headers like this then it becomes a pain to deal with.
> >
> I will configure the corporate mail. I am the server admin , so there should
> not be any problem in settings. :)
>
v4 of the patch was sent from the corporate mail. The settings were done.
But the problem is coming in a different area. I have given strict DMARC check
for the corporate mail server. DMARC = domain based message authentication.
So the mail i sent reached all the list subscriber from a different server than
our designated server , and as a result it has been marked as spam in many places.
I have already received a few complaints regarding that.
Is there any other way that i send the patch from my personal account ,
and use my corporate mail in Signed-off-by ...

thanks
sudip

> > >
> > > removed unused variables
> > > fixed sparse warning of context imbalance in 'do_locked_client_insert'
> > > different lock contexts for basic block
> > >
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > ---
> > >
> >
> > This patch is much better and more interesting, but I still want some
> > more changes.
> >
> I have already sent v3 of the patch just before your mail , based on
> what greg k-h has suggested about the commnent. Please discard that.
>
> > > v1 of the patch of the patch just fixed the sparse warning.
> > > On suggestion of Dan Carpenter v2 is the total rewrite of the function.
> > > Two of the function arguments (interruptHandle,channelId) are also not used. Wanted to remove them as well ,
> > > but then thought maybe the original author have planned for some use of those variables.
> >
> > In the kernel we don't put code in until we are ready to use it. Don't
> > worry about future changes. But on the other hand, don't remove the
> > parameters in this patch because that is doing too many changes in one
> > patch. It would have to be done in a follow on patch if you decide to
> > do it.
> >
> > > - if (locked) {
> > > - spin_unlock_irqrestore((spinlock_t *) lock, flags);
> > > - locked = 0;
> > > + goto unlock;
> > > + visor_signalqueue_empty(queueinfo->chan, whichqueue);
> >
> > Just remove this function. But mention it in the changelog in case
> > there are side effects.
> >
> > > + /*visor_signal_insert() only return 0 or 1 */
> >
> > Don't put obvious comments like this. A normal reader will assume that
> > this function is boolean based on how it is used.
> >
> > > + if (visor_signal_insert(queueinfo->chan, whichqueue, pSignal) == 1) {
> >
> > Don't put the == 1. In terms of English, 1 really is intended as
> > "success" and not the number one. Also don't test for == true or
> > == false.
> >
> > if (foo) {
> > if (foo == true) {
> >
> > These two statement *mean* the same thing in terms of English, but the
> > first one is simpler and less wordy.
> >
> > regards,
> > dan carpenter
>
> thanks
> sudip