[PATCH v2 12/12] os: always check if client is local when connection is accepted

[Pauli Nieminen]

On Thu, Dec 30, 2010 at 9:07 PM, Alan Coopersmith
<alan.coopersmith at oracle.com> wrote:
> On 12/30/10 09:19 AM, Pauli wrote:
>> From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
>>
>> LocalClient is used for all DRI2 requests that makes it frequently
>> called function. Querying if connection is local or not takes 10-15us
>> (on ARM) depending on malloc speed.
>>
>> Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
>> ---
>>  os/access.c     |   34 +---------------------------------
>>  os/connection.c |   39 +++++++++++++++++++++++++++++++++++++++
>>  os/osdep.h      |    1 +
>>  3 files changed, 41 insertions(+), 33 deletions(-)
>>
>> diff --git a/os/access.c b/os/access.c
>> index 494986e..1df62c0 100644
>> --- a/os/access.c
>> +++ b/os/access.c
>> @@ -1030,41 +1030,9 @@ ResetHosts (char *display)
>>  /* Is client on the local host */
>>  Bool LocalClient(ClientPtr client)
>>  {
>> -    int              alen, family, notused;
>> -    Xtransaddr               *from = NULL;
>> -    pointer          addr;
>> -    register HOST    *host;
>> -
>>      if (!client->osPrivate)
>>          return FALSE;
>> -    if (!((OsCommPtr)client->osPrivate)->trans_conn)
>> -        return FALSE;
>> -
>> -    if (!_XSERVTransGetPeerAddr (((OsCommPtr)client->osPrivate)->trans_conn,
>> -     &notused, &alen, &from))
>> -    {
>> -     family = ConvertAddr ((struct sockaddr *) from,
>> -         &alen, (pointer *)&addr);
>> -     if (family == -1)
>> -     {
>> -         free(from);
>> -         return FALSE;
>> -     }
>> -     if (family == FamilyLocal)
>> -     {
>> -         free(from);
>> -         return TRUE;
>> -     }
>> -     for (host = selfhosts; host; host = host->next)
>> -     {
>> -         if (addrEqual (family, addr, alen, host)) {
>> -             free(from);
>> -             return TRUE;
>> -         }
>> -     }
>> -     free(from);
>> -    }
>> -    return FALSE;
>> +    return ((OsCommmPtr)client->osPrivate)->local_client;
>>  }
>>
>>  /*
>> diff --git a/os/connection.c b/os/connection.c
>> index 5452ae1..f545589 100644
>> --- a/os/connection.c
>> +++ b/os/connection.c
>> @@ -718,6 +718,44 @@ ClientAuthorized(ClientPtr client,
>>      return((char *)NULL);
>>  }
>>
>> +static Bool
>> +ComputerLocalClient(OsCommPtr oc)
>
> That should be "Compute" not "Computer"  (surprised it compiled,
> since you called it as Compute below).
>

Most likely it didn't link and I tested my previous version of X :/
But tomorrow is maybe better day.

>> +{
>> +    int              alen, family, notused;
>> +    Xtransaddr               *from = NULL;
>> +    pointer          addr;
>> +    register HOST    *host;
>> +
>> +    if (!oc->trans_conn)
>> +        return FALSE;
>> +
>> +    if (!_XSERVTransGetPeerAddr (oc->trans_conn,
>> +     &notused, &alen, &from))
>> +    {
>> +     family = ConvertAddr ((struct sockaddr *) from,
>> +         &alen, (pointer *)&addr);
>> +     if (family == -1)
>> +     {
>> +         free(from);
>> +         return FALSE;
>> +     }
>> +     if (family == FamilyLocal)
>> +     {
>> +         free(from);
>> +         return TRUE;
>> +     }
>> +     for (host = selfhosts; host; host = host->next)
>> +     {
>> +         if (addrEqual (family, addr, alen, host)) {
>> +             free(from);
>> +             return TRUE;
>> +         }
>> +     }
>> +     free(from);
>> +    }
>> +    return FALSE;
>> +}
>> +
>>  static ClientPtr
>>  AllocNewConnection (XtransConnInfo trans_conn, int fd, CARD32 conn_time)
>>  {
>> @@ -741,6 +779,7 @@ AllocNewConnection (XtransConnInfo trans_conn, int fd, CARD32 conn_time)
>>      oc->output = (ConnectionOutputPtr)NULL;
>>      oc->auth_id = None;
>>      oc->conn_time = conn_time;
>> +    oc->local_client = ComputeLocalClient(oc);
>>      if (!(client = NextAvailableClient((pointer)oc)))
>>      {
>>       free(oc);
>> diff --git a/os/osdep.h b/os/osdep.h
>> index 3c0e78f..b47605e 100644
>> --- a/os/osdep.h
>> +++ b/os/osdep.h
>> @@ -172,6 +172,7 @@ typedef struct _osComm {
>>      XID      auth_id;                /* authorization id */
>>      CARD32 conn_time;                /* timestamp if not established, else 0  */
>>      struct _XtransConnInfo *trans_conn; /* transport connection object */
>> +    Bool local_client;
>>  } OsCommRec, *OsCommPtr;
>>
>>  extern int FlushClient(
>
>
> Everything else looks fine from a quick review though.
>
> --
>        -Alan Coopersmith-        alan.coopersmith at oracle.com
>         Oracle Solaris Platform Engineering: X Window System
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>