Re: [PATCH] biosdisk, getroot for Cygwin

[Robert Millan <rmh@aybabtu.com>]

On Fri, May 09, 2008 at 07:32:42PM +0200, Christian Franke wrote:
> >>+  char devpath[sizeof ("/dev/sda15") + 13];
> >>    
> >
> >Where does this 13 come from?  Would be nice to make it explicit (e.g.
> >sizeof(something) or so).
> 
> 13 "paranoia" bytes added to the required size :-)

A comment saying that would be nice.

On Sun, May 11, 2008 at 11:04:39PM +0200, Christian Franke wrote:
>
> Probably more readable and extensible - Use early returns:

Yeah I like that better too.

> #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'd suggest removing the double-negation here;  it looks more readable
with "#ifdef / #else" IMHO (and is easier to add more OS-specific checks
like "#if defined(__CYGWIN__) / #elif defined(...) / #else".

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)