bind is a valid key for tcp/nonce-tcp transport, however, which only documented in dbus-daemon(1), should be documented in DBus Spec too.
Created attachment 90212 [details] [review] [PATCH 1/3] DBus Spec: fix format a little OK, the format of spec file is rather crazy, just fix a little part of indent before add document for "bind".
Created attachment 90213 [details] [review] [PATCH 2/3] DBus Spec: add document of bind for tcp/nonce-tcp transport tcp/nonce-tcp transport has a "bind" key, which can be specified a hostname and will override hostname specified in "host" key.
Created attachment 90214 [details] [review] [PATCH 3/3] DBus-daemon(1): clarify hostname of bind to avoid confusing At previous, I confused by below string in dbus-daemon(1) The specified host should be a valid name of the local machine or weird stuff will happen. "The specified host" may also understand as "the hostname specified by 'host' key". Maybe this is a problem for me (English is my second language), so to avoid the others got confused, let's clarify it as "The specified hostname of bind".
Comment on attachment 90212 [details] [review] [PATCH 1/3] DBus Spec: fix format a little Review of attachment 90212 [details] [review]: ----------------------------------------------------------------- If you're changing the whitespace at all, I'd prefer 2 spaces per indent, which is as close as the D-Bus spec gets to consistency.
Comment on attachment 90213 [details] [review] [PATCH 2/3] DBus Spec: add document of bind for tcp/nonce-tcp transport Review of attachment 90213 [details] [review]: ----------------------------------------------------------------- I think a better way to express this might be to merge my patch defining listenable/connectable addresses, and say something like this: bind (string) The IP address of one of the local machine's interfaces (most commonly <literal>127.0.0.1</literal>), or a DNS name that resolves to one of those IP addresses, or <literal>0.0.0.0</literal> to listen on all interfaces simultaneously. If not specified, the default is the same value as "host". I assume the purpose here is that if you're doing remote TCP on a machine with addresses 10.0.0.1 and 10.11.12.13, you can tell the dbus-daemon to listen on tcp:host=10.0.0.1,bind=0.0.0.0 and it will report a "recommended" connectable address like tcp:host=10.0.0.1,port=51423 but clients that try to connect to tcp:host=10.11.12.13,port=51423 would also connect successfully? ::: doc/dbus-specification.xml @@ +3128,5 @@ > + <entry>(string)</entry> > + <entry>hostname to bind to, which will override the "host" option > + specifying what address to bind to, without changing the address > + reported by the bus. The bind option can also take a special name > + '*' to cause the bus to listen on all local address (INADDR_ANY). Is the special treatment of "*" something that is hard-coded in libdbus, or is it a quirk of the OS resolver? If it's hard-coded in libdbus, fine. If it's an OS resolver feature ("getent hosts '*'" happens to return "0.0.0.0" on that platform), we shouldn't document or recommend it.
Comment on attachment 90214 [details] [review] [PATCH 3/3] DBus-daemon(1): clarify hostname of bind to avoid confusing Review of attachment 90214 [details] [review]: ----------------------------------------------------------------- ::: doc/dbus-daemon.1.xml.in @@ +396,5 @@ > the host option specifying what address to bind to, without changing > the address reported by the bus. The bind option can also take a > special name '*' to cause the bus to listen on all local address > +(INADDR_ANY). The specified hostname of bind should be a valid > +name of the local machine or it will fail.</para> As in my review of the previous patch, I think the emphasis should be on IP addresses: the value for "bind" is either 0.0.0.0 to listen on all addresses, or one of the machine's IP addresses (typically 127.0.0.1 to listen on the loopback interface), or a hostname (or maybe use "DNS name" if you prefer to avoid the word "host" here) that will resolve to one of the machine's addresses.
(In reply to comment #4) > Comment on attachment 90212 [details] [review] [review] > [PATCH 1/3] DBus Spec: fix format a little > > Review of attachment 90212 [details] [review] [review]: > ----------------------------------------------------------------- > > If you're changing the whitespace at all, I'd prefer 2 spaces per indent, > which is as close as the D-Bus spec gets to consistency. I was trying to find a tool to format dbus-specification.xml, however, xmllint and tidy doesn't only just do indent, which also change the other part I do not expected. So I'd like to left the indent problem but replace '\t' with 8 spaces and another issue I found with tidy is four double quotes (") isn't in english format, here is (“”)
(In reply to comment #5) > Comment on attachment 90213 [details] [review] [review] > [PATCH 2/3] DBus Spec: add document of bind for tcp/nonce-tcp transport > > Review of attachment 90213 [details] [review] [review]: > ----------------------------------------------------------------- > > I think a better way to express this might be to merge my patch defining > listenable/connectable addresses, and say something like this: Sure, I remember I did r+ on that patch? > > bind (string) > The IP address of one of the local machine's interfaces (most commonly > <literal>127.0.0.1</literal>), or a DNS name that resolves to one of those > IP addresses, or <literal>0.0.0.0</literal> to listen on all interfaces > simultaneously. If not specified, the default is the same value as "host". > > I assume the purpose here is that if you're doing remote TCP on a machine > with addresses 10.0.0.1 and 10.11.12.13, you can tell the dbus-daemon to > listen on > > tcp:host=10.0.0.1,bind=0.0.0.0 > > and it will report a "recommended" connectable address like > > tcp:host=10.0.0.1,port=51423 > > but clients that try to connect to tcp:host=10.11.12.13,port=51423 would > also connect successfully? Didn't test in real environment, but I think this is correct, giving that it's the same as a system with a loopback address and a local NIC setup. > > ::: doc/dbus-specification.xml > @@ +3128,5 @@ > > + <entry>(string)</entry> > > + <entry>hostname to bind to, which will override the "host" option > > + specifying what address to bind to, without changing the address > > + reported by the bus. The bind option can also take a special name > > + '*' to cause the bus to listen on all local address (INADDR_ANY). > > Is the special treatment of "*" something that is hard-coded in libdbus, or > is it a quirk of the OS resolver? For key "bind", it's hard-coded in libdbus, so I think we should document it in DBus Spec, in a way prefer to 0.0.0.0 > > If it's hard-coded in libdbus, fine. > > If it's an OS resolver feature ("getent hosts '*'" happens to return > "0.0.0.0" on that platform), we shouldn't document or recommend it.
Created attachment 90331 [details] [review] [PATCH v2 1/3] DBus Spec: replace tab with 8 spaces
Created attachment 90332 [details] [review] [PATCH 2/4] DBus Spec: fix double quotes
Created attachment 90333 [details] [review] [PATCH 3/4] DBus Spec: add document of bind for tcp/nonce-tcp transport
Created attachment 90335 [details] [review] [PATCH 4/4] dbus-daemon(1): align document about "bind" with DBus Spec
Comment on attachment 90332 [details] [review] [PATCH 2/4] DBus Spec: fix double quotes Review of attachment 90332 [details] [review]: ----------------------------------------------------------------- r-, no need for this. U+201C LEFT DOUBLE QUOTATION MARK and U+201D RIGHT DOUBLE QUOTATION MARK are correct English (formally, more correct than U+0022 QUOTATION MARK), and are fine to use for anything that isn't special syntax (e.g. U+0022 is the only non-misleading quote mark for C/C++ string syntax). They're also annoying to type (Shift+AltGr (Compose), Shift+2 ("), Shift+Comma (<) on my UK X11 keyboard layout), so people who aren't pedantic about Unicode don't usually bother :-)
Comment on attachment 90331 [details] [review] [PATCH v2 1/3] DBus Spec: replace tab with 8 spaces Review of attachment 90331 [details] [review]: ----------------------------------------------------------------- This is OK to a point, although it does break "git blame" and similar tools.
Comment on attachment 90333 [details] [review] [PATCH 3/4] DBus Spec: add document of bind for tcp/nonce-tcp transport Review of attachment 90333 [details] [review]: ----------------------------------------------------------------- ::: doc/dbus-specification.xml @@ +3125,5 @@ > </row> > <row> > + <entry>bind</entry> > + <entry>(string)</entry> > + <entry>The IP address of one of the local machine's interfaces You don't actually say what it *does* here. :-) Perhaps something like this? Used in a listenable address to configure the interface on which the server will listen: either the IP address of [... the rest of what you said ...] @@ +3205,5 @@ > </row> > <row> > + <entry>bind</entry> > + <entry>(string)</entry> > + <entry>The IP address of one of the local machine's interfaces How about just saying "the same as for tcp: addresses" for nonce-tcp: addresses? (There's not enough context in the diff to tell which is which - I'm assuming tcp: comes first.)
Comment on attachment 90335 [details] [review] [PATCH 4/4] dbus-daemon(1): align document about "bind" with DBus Spec Review of attachment 90335 [details] [review]: ----------------------------------------------------------------- ::: doc/dbus-daemon.1.xml.in @@ +392,4 @@ > <para>Example: <listen>tcp:host=localhost,port=0</listen></para> > > > +<para>tcp/nonce-tcp addresses also allow a bind=hostname option, Again, you're saying this is allowed but haven't actually said what it does.
Created attachment 90546 [details] [review] [PATCH v2 2/3] DBus Spec: add document of bind for tcp/nonce-tcp transport
Created attachment 90547 [details] [review] [PATCH v2 3/3] dbus-daemon(1): align document about "bind" with DBus Spec
Fixed in git for 1.7.10, thanks
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.