You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/12/12 13:49:20 UTC

Re: cvs commit: apr/file_io/unix filestat.c

wrowe@apache.org writes:

> wrowe       2002/12/11 23:01:52
> 
>   Modified:    file_io/unix filestat.c
>   Log:
>     switch {case} and default: are probably better for handling this case.
>     Is anyone aware of a platform where S_IFxxx # isn't available, yet
>     S_ISxxx(#) is?

RH Linux 7.3 doesn't have S_IFFIFO

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: apr/file_io/unix filestat.c

Posted by Jeff Trawick <tr...@attglobal.net>.
"William A. Rowe, Jr." <wr...@apache.org> writes:

> At 06:49 AM 12/12/2002, Jeff Trawick wrote:
> >wrowe@apache.org writes:
> >
> >> wrowe       2002/12/11 23:01:52
> >> 
> >>   Modified:    file_io/unix filestat.c
> >>   Log:
> >>     switch {case} and default: are probably better for handling this case.
> >>     Is anyone aware of a platform where S_IFxxx # isn't available, yet
> >>     S_ISxxx(#) is?
> >
> >RH Linux 7.3 doesn't have S_IFFIFO
> 
> Yet it does have S_ISFIFO?

of course, the old code compiled fine everywhere

AIX and Tru64 have reported back by now...  broken build there for the
same reason...

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: apr/file_io/unix filestat.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 09:28 AM 12/12/2002, Brane wrote:
>William A. Rowe, Jr. wrote:
>
>>At 06:49 AM 12/12/2002, Jeff Trawick wrote:
>>  
>>
>>>wrowe@apache.org writes:
>>>
>>>    
>>>
>>>>wrowe       2002/12/11 23:01:52
>>>>
>>>>  Modified:    file_io/unix filestat.c
>>>>  Log:
>>>>    switch {case} and default: are probably better for handling this case.
>>>>    Is anyone aware of a platform where S_IFxxx # isn't available, yet
>>>>    S_ISxxx(#) is?
>>>>      
>>>>
>>>RH Linux 7.3 doesn't have S_IFFIFO
>>
>>Yet it does have S_ISFIFO?
>
>The funny thing about this change is that, for a short switch like that,
>many modern compilers just convert it to a series of "if ... else if"
>tests. So just putting the "else"s in there wouldn've been better -- not
>to mention more correct. I don't think generating a jump table for just
>seven cases makes sense on any platform.

While I agree with you if the tests...

if (i == S_IFxxx)      if (S_IFxxx(i))

were identical, they aren't.  This change still gains us the potential benefit
(possibly optimized away) of not remasking the type from the mode repeatedly.

I've worked around missing S_IFFIFO and S_IFSOCK.  If there are others
on other odd platforms, I'll surrender, revert the patch and add the missing
else-es (in the better order they are given now; reg, dir, lnk etc.)

Bill




Re: cvs commit: apr/file_io/unix filestat.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 09:28 AM 12/12/2002, Brane wrote:
>William A. Rowe, Jr. wrote:
>
>>At 06:49 AM 12/12/2002, Jeff Trawick wrote:
>>  
>>
>>>wrowe@apache.org writes:
>>>
>>>    
>>>
>>>>wrowe       2002/12/11 23:01:52
>>>>
>>>>  Modified:    file_io/unix filestat.c
>>>>  Log:
>>>>    switch {case} and default: are probably better for handling this case.
>>>>    Is anyone aware of a platform where S_IFxxx # isn't available, yet
>>>>    S_ISxxx(#) is?
>>>>      
>>>>
>>>RH Linux 7.3 doesn't have S_IFFIFO
>>
>>Yet it does have S_ISFIFO?
>
>The funny thing about this change is that, for a short switch like that,
>many modern compilers just convert it to a series of "if ... else if"
>tests. So just putting the "else"s in there wouldn've been better -- not
>to mention more correct. I don't think generating a jump table for just
>seven cases makes sense on any platform.

While I agree with you if the tests...

if (i == S_IFxxx)      if (S_IFxxx(i))

were identical, they aren't.  This change still gains us the potential benefit
(possibly optimized away) of not remasking the type from the mode repeatedly.

I've worked around missing S_IFFIFO and S_IFSOCK.  If there are others
on other odd platforms, I'll surrender, revert the patch and add the missing
else-es (in the better order they are given now; reg, dir, lnk etc.)

Bill




Re: cvs commit: apr/file_io/unix filestat.c

Posted by Branko Čibej <br...@xbc.nu>.
William A. Rowe, Jr. wrote:

>At 06:49 AM 12/12/2002, Jeff Trawick wrote:
>  
>
>>wrowe@apache.org writes:
>>
>>    
>>
>>>wrowe       2002/12/11 23:01:52
>>>
>>>  Modified:    file_io/unix filestat.c
>>>  Log:
>>>    switch {case} and default: are probably better for handling this case.
>>>    Is anyone aware of a platform where S_IFxxx # isn't available, yet
>>>    S_ISxxx(#) is?
>>>      
>>>
>>RH Linux 7.3 doesn't have S_IFFIFO
>>    
>>
>
>Yet it does have S_ISFIFO?
>
>Bill
>

The funny thing about this change is that, for a short switch like that,
many modern compilers just convert it to a series of "if ... else if"
tests. So just putting the "else"s in there wouldn've been better -- not
to mention more correct. I don't think generating a jump table for just
seven cases makes sense on any platform.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


Re: cvs commit: apr/file_io/unix filestat.c

Posted by Branko Čibej <br...@xbc.nu>.
William A. Rowe, Jr. wrote:

>At 06:49 AM 12/12/2002, Jeff Trawick wrote:
>  
>
>>wrowe@apache.org writes:
>>
>>    
>>
>>>wrowe       2002/12/11 23:01:52
>>>
>>>  Modified:    file_io/unix filestat.c
>>>  Log:
>>>    switch {case} and default: are probably better for handling this case.
>>>    Is anyone aware of a platform where S_IFxxx # isn't available, yet
>>>    S_ISxxx(#) is?
>>>      
>>>
>>RH Linux 7.3 doesn't have S_IFFIFO
>>    
>>
>
>Yet it does have S_ISFIFO?
>
>Bill
>

The funny thing about this change is that, for a short switch like that,
many modern compilers just convert it to a series of "if ... else if"
tests. So just putting the "else"s in there wouldn've been better -- not
to mention more correct. I don't think generating a jump table for just
seven cases makes sense on any platform.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


Re: cvs commit: apr/file_io/unix filestat.c

Posted by Jeff Trawick <tr...@attglobal.net>.
"William A. Rowe, Jr." <wr...@apache.org> writes:

> At 06:49 AM 12/12/2002, Jeff Trawick wrote:
> >wrowe@apache.org writes:
> >
> >> wrowe       2002/12/11 23:01:52
> >> 
> >>   Modified:    file_io/unix filestat.c
> >>   Log:
> >>     switch {case} and default: are probably better for handling this case.
> >>     Is anyone aware of a platform where S_IFxxx # isn't available, yet
> >>     S_ISxxx(#) is?
> >
> >RH Linux 7.3 doesn't have S_IFFIFO
> 
> Yet it does have S_ISFIFO?

of course, the old code compiled fine everywhere

AIX and Tru64 have reported back by now...  broken build there for the
same reason...

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: apr/file_io/unix filestat.c

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 06:49 AM 12/12/2002, Jeff Trawick wrote:
>wrowe@apache.org writes:
>
>> wrowe       2002/12/11 23:01:52
>> 
>>   Modified:    file_io/unix filestat.c
>>   Log:
>>     switch {case} and default: are probably better for handling this case.
>>     Is anyone aware of a platform where S_IFxxx # isn't available, yet
>>     S_ISxxx(#) is?
>
>RH Linux 7.3 doesn't have S_IFFIFO

Yet it does have S_ISFIFO?

Bill