linux-kernel - Re: [PATCH v4 10/13] xen/pvcalls: implement recvmsg

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:   Fri, 6 Oct 2017 13:46:02 -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 v4 10/13] xen/pvcalls: implement recvmsg

On Fri, 22 Sep 2017, Boris Ostrovsky wrote:
> > +static bool pvcalls_front_read_todo(struct sock_mapping *map)
> > +{
> > +	struct pvcalls_data_intf *intf = map->active.ring;
> > +	RING_IDX cons, prod;
> > +	int32_t error;
> > +
> > +	cons = intf->in_cons;
> > +	prod = intf->in_prod;
> > +	error = intf->in_error;
> > +	return (error != 0 ||
> > +		pvcalls_queued(prod, cons,
> > +			       XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER)) != 0);
> 
> 
> Does this routine have to be different from pvcalls_front_write_todo()?
> They look pretty similar. Can they be merged?
> 
> (and you don't really need 'error' variable)

pvcalls_front_write_todo and pvcalls_front_read_todo are both small
wrappers around pvcalls_queued. However, they check different indexes.
A single function would look like this:

  static bool pvcalls_front_todo(struct sock_mapping *map, bool write)
  {
  	struct pvcalls_data_intf *intf = map->active.ring;
  	RING_IDX cons, prod, size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
  	int32_t error;
  	RING_IDX q;
  	bool ret;
  
  	if (write) {
  		cons = intf->out_cons;
  		prod = intf->out_prod;
  		error = intf->out_error;
  	} else {
  		cons = intf->in_cons;
  		prod = intf->in_prod;
  		error = intf->in_error;
  	}
  	if (error == -ENOTCONN)
  		return false;
  	if (error != 0)
  		return true;
  
  	q = pvcalls_queued(prod, cons, size);
  	if (write)
  		ret = !!(size - q);
  	else
  		ret = q != 0;
  	return ret;
  }

I don't think is much of an improvement?