apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mladen Turk <mt...@apache.org>
Subject Re: svn commit: r745763 - in /apr/apr/trunk: ./ build/ include/ include/arch/unix/ network_io/unix/ test/
Date Fri, 20 Feb 2009 07:01:54 GMT
Ruediger Pluem wrote:
> 
>> +    if (family == APR_UNSPEC && hostname && *hostname == '/')
>> +        family = APR_UNIX;
>> +    if (family == APR_UNIX) {
>> +#if APR_HAVE_SOCKADDR_UN
>> +        if (hostname && *hostname == '/') {
> 
> Why is it needed that hostname always starts with a '/'?
>

Right, the second test is not needed.
The first one with APR_UNSPEC is, cause that's the
only (simple enough) way to determine that the address is AF_UNIX one.

>> +#if APR_HAVE_SOCKADDR_UN
>> +    else if (sockaddr->family == APR_UNIX) {
>> +        *hostname = sockaddr->hostname;
> 
> Shouldn't we do
> 
> apr_pstrdup(sockaddr->pool, sockaddr->hostname)
> 
> to protect our internal hostname in sockaddr?
>

Hmm, probably.

>>  
>> -static char generic_inaddr_any[16] = {0}; /* big enough for IPv4 or IPv6 */
>> +#if APR_HAVE_SOCKADDR_UN
>> +#define GENERIC_INADDR_ANY_LEN  sizeof(struct sockaddr_un)
> 
> Do we know that sizeof(struct sockaddr_un) is always >= 16?
>

Think that's always true. It's always > 100
We can use APR_ALIGN(sizeof(struct sockaddr_un), 16)
if you think that's not the case.

>> +#else
>> +#define GENERIC_INADDR_ANY_LEN  16
>> +#endif
>> +
>> +/* big enough for IPv4, IPv6 and optionaly sun_path */
>> +static char generic_inaddr_any[GENERIC_INADDR_ANY_LEN] = {0};
>>  
>>  static apr_status_t socket_cleanup(void *sock)
>>  {
>>      apr_socket_t *thesocket = sock;
>>  
>> +#if APR_HAVE_SOCKADDR_UN
>> +    if (thesocket->bound && thesocket->local_addr->family == APR_UNIX)
{
>> +        /* XXX: Check for return values ? */
>> +        unlink(thesocket->local_addr->hostname);
> 
> Shouldn't we close the socket before unlinking?
> 

IMO that's the same.
<snip>
If the name referred to a socket, fifo or device the name for it
is removed but processes which have the object open may continue
to use it.
</snip>

Thus IMO it's always better first to unlink and then close,
preventing race conditions (hard to happen in this tight loop
anyhow, but ...).

Regards
-- 
^(TM)

Mime
View raw message