Re: [PATCH] biosdisk, getroot for Cygwin

[Christian Franke <Christian.Franke@t-online.de>]

Christian Franke wrote:
> Robert Millan wrote:
>> On Wed, May 07, 2008 at 10:42:23PM +0200, Christian Franke wrote:
>>> +#ifndef __CYGWIN__
>>>  #ifdef __linux__
>>>    /* We first try to find the device in the /dev/mapper directory.  If
>>>       we don't do this, we get useless device names like /dev/dm-0 for
>>> @@ -242,12 +373,19 @@ grub_guess_root_device (const char *dir)
>>>        os_dev = find_root_device ("/dev", st.st_dev);
>>>      }
>>>  
>>> +#else
>>> +  /* Cygwin specific function.  */
>>> +  os_dev = find_cygwin_root_device (dir, st.st_dev);
>>> +
>>> +#endif /* __CYGWIN__ */
>>>     
>>
>> The logic here seems a bit over-complicated.  Surely whenever you 
>> have __linux__
>> you don't have __CYGWIN__.  Are you sure it can't be simplified?
>>
>>   
>
> An alternative would be to move the #ifndef __CYGWIN__ behind the 
> #ifdef __linux__ block:
>
> #ifdef __linux__
> ...
>  os_dev = find_root_device ("/dev/mapper", st.st_dev);
> ...
>  if (! os_dev)
> #endif
> #ifndef __CYGWIN__
>    {
>      /* This might be truly slow, but is there any better way?  */
>      os_dev = find_root_device ("/dev", st.st_dev);
>    }
>
> #else /* __CYGWIN__ */
>  /* Cygwin specific function.  */
>  os_dev = find_cygwin_root_device (dir, st.st_dev);
>
> #endif /* __CYGWIN__ */
>
>
> I'm not sure whether this would be more readable.
>


Probably more readable and extensible - Use early returns:

#ifdef __linux__
  /* We first try to find the device in the /dev/mapper directory.  If
     we don't do this, we get useless device names like /dev/dm-0 for
     LVM.  */
  os_dev = find_root_device ("/dev/mapper", st.st_dev);
  if (os_dev)
    return os_dev;

  /* The same applies to /dev/evms directory (for EVMS volumes).  */
  os_dev = find_root_device ("/dev/evms", st.st_dev);
  if (os_dev)
    return os_dev;
#endif /* __linux__ */

#ifndef __CYGWIN__
  /* This might be truly slow, but is there any better way?  */
  os_dev = find_root_device ("/dev", st.st_dev);

#else /* __CYGWIN__ */
  /* Cygwin specific function.  */
  os_dev = find_cygwin_root_device (dir, st.st_dev);

#endif /* __CYGWIN__ */

  return os_dev;
}


Christian