linux-kernel - Re: [PATCH v2 05/13] xen/pvcalls: implement bind command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 31 Jul 2017 15:17:17 -0700 (PDT)
From:   Stefano Stabellini <sstabellini@...nel.org>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>
cc:     Stefano Stabellini <sstabellini@...nel.org>,
        xen-devel@...ts.xen.org, linux-kernel@...r.kernel.org,
        jgross@...e.com, Stefano Stabellini <stefano@...reto.com>
Subject: Re: [PATCH v2 05/13] xen/pvcalls: implement bind command

On Thu, 27 Jul 2017, Boris Ostrovsky wrote:
> >> This all looks very similar to previous patches. Can it be factored out?
> > You are right that the pattern is the same for all commands:
> > - get a request
> > - fill the request
> > - possibly do something else
> > - wait
> > however each request is different, the struct and fields are different.
> > There are spin_lock and spin_unlock calls intermingled. I am not sure I
> > can factor out much of this. Maybe I could create a static inline or
> > macro as a syntactic sugar to replace the wait call, but that's pretty
> > much it I think.
> 
> Maybe you could factor out common fragments, not necessarily the whole
> thing at once?
> 
> For example,
> 
> static inline int get_request(*bedata, int *req_id)
> {
> 
> 	*req_id = bedata->ring.req_prod_pvt & (RING_SIZE(&bedata->ring) - 1);
> 	if (RING_FULL(&bedata->ring) ||
> 	    READ_ONCE(bedata->rsp[*req_id].req_id) != PVCALLS_INVALID_ID) {
> 		return -EAGAIN;
> 	return 0;
> }
> 
> (or some such)

You are right, the code looks better this way. I'll add it.