Re: [PATCH v3 1/2] Add grub_util_readlink()

[Daniel Kiper]

On Fri, Dec 13, 2019 at 03:44:21PM +0100, Javier Martinez Canillas wrote:
> Hello Daniel,
>
> On 11/5/19 12:42 PM, Daniel Kiper wrote:
>
> [snip]
>
> >> --- grub-core/osdep/windows/hostdisk.c
> >> +++ grub-core/osdep/windows/hostdisk.c
> >> @@ -365,6 +365,12 @@ grub_util_mkdir (const char *dir)
> >>    free (windows_name);
> >>  }
> >>
> >> +ssize_t
> >> +grub_util_readlink (const char *name, char *buf, size_t bufsize)
> >> +{
> >> +  return readlink(name, buf, bufsize);
> >> +}
> >> +
> >>  int
> >>  grub_util_rename (const char *from, const char *to)
> >>  {
> >
> > This change breaks Windows build. I think that you should drop it and
> > do some #ifdefery in patch #2. I mean I would leave existing code for
> > Windows only and add new code for POSIX compliant systems.
> >
>
> Ok, will do.
>
> [snip]
>
> >>  #if defined (__NetBSD__)
> >> diff --git include/grub/osdep/hostfile_windows.h 
> >> include/grub/osdep/hostfile_windows.h
> >> index bf6451b6db4..8c92d0591bb 100644
> >> --- include/grub/osdep/hostfile_windows.h
> >> +++ include/grub/osdep/hostfile_windows.h
> >> @@ -41,6 +41,8 @@ typedef struct grub_util_fd_dir *grub_util_fd_dir_t;
> >>
> >>  int
> >>  grub_util_rename (const char *from, const char *to);
> >> +ssize_t
> >> +grub_util_readlink (const char *name, char *buf, size_t bufsize);
> >>  int
> >>  grub_util_unlink (const char *name);
> >>  void
> >
> > This change is also probably not needed then...
> >
>
> Do we really need this grub_util_readlink() indirection level then?
> Can the #ifdef code for POSIX just use readlink(2) directly?

I think that yes/no because after dropping Windows code you still have two
variants. So, I would leave this grub_util_readlink() indirection as is.

Daniel