default tcp host

[Daniel P. Berrange]

On Wed, Jun 20, 2007 at 12:46:00PM -0400, Havoc Pennington wrote:
> Daniel P. Berrange wrote:
> >+#define MAX_SOCKET 10
> 
> Just sucking it up and dynamically sizing this array probably makes 
> sense - it never resizes, the nfd is passed in at construct-time, so 
> it's just as easy.

Done - its fully dynamic now.

> 
> >+  int nfd;                       /**< Number of active file handles */
> 
> ("n_fds" would be a more typical variable name in the dbus codebase)
> 
> >+  for (i = 0 ; i < socket_server->nfd ; i++)
> >+    if (socket_server->watch[i])
> >+      {
> >+	_dbus_watch_unref (socket_server->watch[i]);
> >+	socket_server->watch[i] = NULL;
> >+      }
> 
> Since this indented wrong in the patch/email there may be some tabs 
> involved; I have been meaning to put indent-tabs-mode: nil in the emacs 
> magic, but in the meantime or if you use vi, other means of avoiding 
> tabs would be good. (I know the existing codebase has a bunch of them, 
> but I'd rather it didn't)

I didn't realize there was a no-tabs policy, since there's plenty of 
code in dbus/*.c using tabs. I've stripped out all tabs from the code 
I've added myself in this latest patch.

> >+  int found = 0, i;
> 
> For a boolean use
> dbus_bool_t found = FALSE

This is changed.

> 
> >   SERVER_LOCK (server);
> >   
> >-  _dbus_assert (watch == socket_server->watch);
> >+  for (i = 0 ; i < socket_server->nfd ; i++)
> >+    {
> >+      if (socket_server->watch[i] == watch)
> >+	found = 1;
> >+    }
> >+  _dbus_assert (found);
> 
> If an assert requires a block like this, do:
> 
> #ifndef DBUS_DISABLE_ASSERT
> {
>    dbus_bool_t found;
>    int i;
>    found = FALSE;
>    for (i = 0; ...
>    _dbus_assert(found);
> }
> #endif

Done.

> >+  else if (!strcmp(bind, "*"))
> 
> The convention in libdbus is == 0 rather than !

Done.

> 
> >+ * @param retfd location to store returned file descriptors
> >+ * @param nretfd maximum number of returnable file descriptors
> 
> Rather than create this limitation / burden on the caller, just do it like:
>  int **fds_p,
>  int  *n_fds_p
> and return an allocated array.
> 
> Similarly for the returned port, though in that case you can have the 
> caller pass in an initialized DBusString to be modified, instead of 
> manually returning a char*

Changed to use dynamic array, and a DBusString.

> Since listen_tcp_socket / connect_tcp_socket are fairly complex now, it 
> may be worth thinking about how some of the logic can be moved 
> cross-platform so it doesn't have to be synced to Windows, though feel 
> free to punt on that and we'll figure it out later. I don't want to do 
> it if it results in a big mess, and I can't really think of a good 
> non-big-mess way to deal with this. Once there is a Windows 
> implementation it might be easier to see how much of it is common to the 
> platforms.

I've not yet tried fixing up the windows side of dbus-sysdeps-win32.c

Aside the fixes I mentioned above, I've re-done some of the error
handling patches to use goto & progressive cleanup at the end of the
function which reduced the lines of code duplication very significantly.

I've also added support for an extra address string attribute 'family=ipv4'
or 'family=ipv6' if someone wants to restrict to a particular IP stack
even on a multi-stack machine - the default is to 'do the right thing',
ie let getaddrinfo return all addresses as it sees fit, filtered based
on config of the host's IP stacks.

Regards,
Dan.
-- 
|=-            GPG key: http://www.berrange.com/~dan/gpgkey.txt       -=|
|=-       Perl modules: http://search.cpan.org/~danberr/              -=|
|=-           Projects: http://freshmeat.net/~danielpb/               -=|
|=-   berrange at redhat.com  -  Daniel Berrange  -  dan at berrange.com    -=|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-getaddrinfo-6.patch
Type: text/x-diff
Size: 35799 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20070723/95fd197d/attachment-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/dbus/attachments/20070723/95fd197d/attachment-0001.pgp