xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Igor Druzhinin <igor.druzhinin@citrix.com>
To: Christian Lindig <christian.lindig@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@citrix.com>, "wl@xen.org" <wl@xen.org>,
	David Scott <dave@recoil.org>
Subject: Re: [Xen-devel] [PATCH] tools/oxenstored: port XS_INTRODUCE evtchn rebind function from cxenstored
Date: Tue, 20 Aug 2019 13:45:05 +0100	[thread overview]
Message-ID: <52ddaa4f-bc1a-43b2-a995-b9e7cae3044f@citrix.com> (raw)
In-Reply-To: <9324516A-66F9-47FC-AD8F-BF44059D29B2@citrix.com>

On 20/08/2019 13:11, Christian Lindig wrote:
> 
> 
>> On 20 Aug 2019, at 11:45, Igor Druzhinin <igor.druzhinin@citrix.com> wrote:
>>
>> On 20/08/2019 09:21, Christian Lindig wrote:
>>>> +			if (Domain.get_mfn edom) = mfn && (Connections.find_domain cons domid) != con then begin
>>>
>>> This should use <> instead of != because != is pointer inequality in OCaml. The parentheses are not strictly necessary because function application has precedence. So:
>>>
>>> 	if Domain.get_mfn edom = mfn && Connections.find_domain cons domid <> con then begin
>>>
>>
>> But I actually want to compare pointers here - the idea is that the
>> connection object in the hashtable indexed by domid is not the same as
>> connection that we got XS_INTRODUCE message from. (see
>> tools/xenstore/xenstrored_domain.c)
> 
> In your code, a != b is true, if a and b have identical structure but different addresses. I strongly suspect that two connection values should have different structure to be considered different, not just different addresses. When a <> b is true, it implies a != b. So using a <> b is safe(r). By using != you would rely on an invariant that every connection (con) exists only once and is never copied.
> 

Connection is a complex object (has various counters and internal
structures) and I don't think simple field by field comparison (is <>
just that in Ocaml?) is appropriate here.

con is passed into that function from xenstored.ml where it's extracted
from the same hashtable (Connections) so it's actually the exactly same
object. There are other examples in connections.ml where pointer
comparison is used to compare connections.

Igor

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2019-08-20 12:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 18:45 [Xen-devel] [PATCH] tools/oxenstored: port XS_INTRODUCE evtchn rebind function from cxenstored Igor Druzhinin
2019-08-20  8:21 ` Christian Lindig
2019-08-20 10:45   ` Igor Druzhinin
2019-08-20 12:11     ` Christian Lindig
2019-08-20 12:45       ` Igor Druzhinin [this message]
2019-08-20 13:09         ` Christian Lindig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52ddaa4f-bc1a-43b2-a995-b9e7cae3044f@citrix.com \
    --to=igor.druzhinin@citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=christian.lindig@citrix.com \
    --cc=dave@recoil.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).