default tcp host

[Havoc Pennington]

Hi,

Awesome, some mostly style-type comments inline, I think the basic 
approach should work fine

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.

> +  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)

> +  int found = 0, i;

For a boolean use
dbus_bool_t found = FALSE

>    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

> +  else if (!strcmp(bind, "*"))

The convention in libdbus is == 0 rather than !

> + * @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*

> +	      strncpy(retport, port, retportlen-1);
> +	      retport[retportlen-1] = '\0';

DBusString is preferred to any manual string manipulation, basically 
char* is only used for "immutable-ish" strings (precision guidelines r 
us!). The codebase avoids most of string.h to minimize that category of 
security (and crash) bugs. (For read-only use, you can quickly init a 
DBusString with an existing char* with _dbus_string_init_const())

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.

Havoc