grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Felix Janda]

Hello,

the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
which gets also pulled in for other libcs (such as musl libc) and makes
compilation fail for them because the code uses glibc internals. The
easiest way to fix is just to remove the code as the below patch does.

--Felix

diff -ur a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
--- a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
+++ b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
@@ -48,11 +48,6 @@
 #ifdef __linux__
 # include <sys/ioctl.h>         /* ioctl */
 # include <sys/mount.h>
-# if !defined(__GLIBC__) || \
-        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
-/* Maybe libc doesn't have large file support.  */
-#  include <linux/unistd.h>     /* _llseek */
-# endif /* (GLIBC < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR < 1)) */
 #endif /* __linux__ */
 
 grub_uint64_t
@@ -79,24 +74,6 @@
   return st.st_size;
 }
 
-#if defined(__linux__) && (!defined(__GLIBC__) || \
-        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1))))
-  /* Maybe libc doesn't have large file support.  */
-int
-grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
-{
-  loff_t offset, result;
-  static int _llseek (uint filedes, ulong hi, ulong lo,
-		      loff_t *res, uint wh);
-  _syscall5 (int, _llseek, uint, filedes, ulong, hi, ulong, lo,
-	     loff_t *, res, uint, wh);
-
-  offset = (loff_t) off;
-  if (_llseek (fd, offset >> 32, offset & 0xffffffff, &result, SEEK_SET))
-    return -1;
-  return GRUB_ERR_NONE;
-}
-#else
 int
 grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
 {
@@ -107,7 +84,6 @@
 
   return 0;
 }
-#endif
 
 
 /* Read LEN bytes from FD in BUF. Return less than or equal to zero if an

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Felix Janda]

This was badly coordinated. Please use the recent patch from daggs instead.

Felix

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Andrei Borzenkov]

В Fri, 21 Nov 2014 21:00:41 +0100
Felix Janda <felix.janda@posteo.de> пишет:

> Hello,
> 
> the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
> which gets also pulled in for other libcs (such as musl libc) and makes
> compilation fail for them because the code uses glibc internals. The
> easiest way to fix is just to remove the code as the below patch does.
> 

Could you replace glibc version check with configure test for llseek?
This should cover your case as well.

> --Felix
> 
> diff -ur a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> --- a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> +++ b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> @@ -48,11 +48,6 @@
>  #ifdef __linux__
>  # include <sys/ioctl.h>         /* ioctl */
>  # include <sys/mount.h>
> -# if !defined(__GLIBC__) || \
> -        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> -/* Maybe libc doesn't have large file support.  */
> -#  include <linux/unistd.h>     /* _llseek */
> -# endif /* (GLIBC < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR < 1)) */
>  #endif /* __linux__ */
>  
>  grub_uint64_t
> @@ -79,24 +74,6 @@
>    return st.st_size;
>  }
>  
> -#if defined(__linux__) && (!defined(__GLIBC__) || \
> -        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1))))
> -  /* Maybe libc doesn't have large file support.  */
> -int
> -grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
> -{
> -  loff_t offset, result;
> -  static int _llseek (uint filedes, ulong hi, ulong lo,
> -		      loff_t *res, uint wh);
> -  _syscall5 (int, _llseek, uint, filedes, ulong, hi, ulong, lo,
> -	     loff_t *, res, uint, wh);
> -
> -  offset = (loff_t) off;
> -  if (_llseek (fd, offset >> 32, offset & 0xffffffff, &result, SEEK_SET))
> -    return -1;
> -  return GRUB_ERR_NONE;
> -}
> -#else
>  int
>  grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
>  {
> @@ -107,7 +84,6 @@
>  
>    return 0;
>  }
> -#endif
>  
>  
>  /* Read LEN bytes from FD in BUF. Return less than or equal to zero if an
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Felix Janda]

Andrei Borzenkov wrote:
> В Fri, 21 Nov 2014 21:00:41 +0100
> Felix Janda <felix.janda@posteo.de> пишет:
> 
> > Hello,
> > 
> > the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
> > which gets also pulled in for other libcs (such as musl libc) and makes
> > compilation fail for them because the code uses glibc internals. The
> > easiest way to fix is just to remove the code as the below patch does.
> > 
> 
> Could you replace glibc version check with configure test for llseek?
> This should cover your case as well.

There is no llseek.

Testing for lseek will likely also succeed for old glibc. I think that
one rather needs to check whether its prototype is correct.

I don't think that the code behind !defined(__GLIBC__) ... is useful
for anything other than glibc<2.1. It seems to use old glibc internals.
(Notice that glibc-2.1 is from 1999.) If the code should be kept in,
it should IMO be conditional on 

((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))


Felix

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]

On 21.11.2014 22:00, Felix Janda wrote:
> Hello,
> 
> the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
> which gets also pulled in for other libcs (such as musl libc) and makes
> compilation fail for them because the code uses glibc internals. The
> easiest way to fix is just to remove the code as the below patch does.
> 
Don't assume that nobody will try to compile with ancient glibc. If you
want to raise the supported version for glibc you need to add a
configure check for presence of long offsets to configure and fail it if
it's not available. It should also be documented in INSTALL.
> --Felix
> 
> diff -ur a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> --- a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> +++ b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
> @@ -48,11 +48,6 @@
>  #ifdef __linux__
>  # include <sys/ioctl.h>         /* ioctl */
>  # include <sys/mount.h>
> -# if !defined(__GLIBC__) || \
> -        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> -/* Maybe libc doesn't have large file support.  */
> -#  include <linux/unistd.h>     /* _llseek */
> -# endif /* (GLIBC < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR < 1)) */
>  #endif /* __linux__ */
>  
>  grub_uint64_t
> @@ -79,24 +74,6 @@
>    return st.st_size;
>  }
>  
> -#if defined(__linux__) && (!defined(__GLIBC__) || \
> -        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1))))
> -  /* Maybe libc doesn't have large file support.  */
> -int
> -grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
> -{
> -  loff_t offset, result;
> -  static int _llseek (uint filedes, ulong hi, ulong lo,
> -		      loff_t *res, uint wh);
> -  _syscall5 (int, _llseek, uint, filedes, ulong, hi, ulong, lo,
> -	     loff_t *, res, uint, wh);
> -
> -  offset = (loff_t) off;
> -  if (_llseek (fd, offset >> 32, offset & 0xffffffff, &result, SEEK_SET))
> -    return -1;
> -  return GRUB_ERR_NONE;
> -}
> -#else
>  int
>  grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)
>  {
> @@ -107,7 +84,6 @@
>  
>    return 0;
>  }
> -#endif
>  
>  
>  /* Read LEN bytes from FD in BUF. Return less than or equal to zero if an
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Felix Janda]

On 28.11.2014, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 21.11.2014 22:00, Felix Janda wrote:
> > Hello,
> > 
> > the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
> > which gets also pulled in for other libcs (such as musl libc) and makes
> > compilation fail for them because the code uses glibc internals. The
> > easiest way to fix is just to remove the code as the below patch does.
> > 
> Don't assume that nobody will try to compile with ancient glibc. If you
> want to raise the supported version for glibc you need to add a
> configure check for presence of long offsets to configure and fail it if
> it's not available. It should also be documented in INSTALL.

Before messing around with configure.ac, I'd like know whether a patch
(correctly) implementing what you suggest would be accepted.

Another solution I would be happy with, is to change the conditions

# if !defined(__GLIBC__) || \
        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))

to

# ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))

Felix

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Andrei Borzenkov]

В Thu, 4 Dec 2014 23:21:09 +0100
Felix Janda <felix.janda@posteo.de> пишет:

> On 28.11.2014, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > On 21.11.2014 22:00, Felix Janda wrote:
> > > Hello,
> > > 
> > > the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
> > > which gets also pulled in for other libcs (such as musl libc) and makes
> > > compilation fail for them because the code uses glibc internals. The
> > > easiest way to fix is just to remove the code as the below patch does.
> > > 
> > Don't assume that nobody will try to compile with ancient glibc. If you
> > want to raise the supported version for glibc you need to add a
> > configure check for presence of long offsets to configure and fail it if
> > it's not available. It should also be documented in INSTALL.
> 
> Before messing around with configure.ac, I'd like know whether a patch
> (correctly) implementing what you suggest would be accepted.
> 
> Another solution I would be happy with, is to change the conditions
> 
> # if !defined(__GLIBC__) || \
>         ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> 
> to
> 
> # ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> 

This will likely result in build error if __GLIBC__ is undefined.

> Felix
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Felix Janda]

Andrei Borzenkov wrote:
> В Thu, 4 Dec 2014 23:21:09 +0100
> Felix Janda <felix.janda@posteo.de> пишет:
[..]
> > Another solution I would be happy with, is to change the conditions
> > 
> > # if !defined(__GLIBC__) || \
> >         ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> > 
> > to
> > 
> > # ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> > 
> 
> This will likely result in build error if __GLIBC__ is undefined.

Right, the condition is wrong if __GLIBC__ is undefined. How about
the below patch?

Felix

--- a/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
+++ b/grub-2.02~beta2/grub-core/osdep/unix/hostdisk.c
@@ -48,7 +48,7 @@
 #ifdef __linux__
 # include <sys/ioctl.h>         /* ioctl */
 # include <sys/mount.h>
-# if !defined(__GLIBC__) || \
+# if defined(__GLIBC__) && \
         ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
 /* Maybe libc doesn't have large file support.  */
 #  include <linux/unistd.h>     /* _llseek */
@@ -79,8 +79,8 @@
   return st.st_size;
 }
 
-#if defined(__linux__) && (!defined(__GLIBC__) || \
-        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1))))
+#if defined(__linux__) && defined(__GLIBC__) && \
+        ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
   /* Maybe libc doesn't have large file support.  */
 int
 grub_util_fd_seek (grub_util_fd_t fd, grub_uint64_t off)

Re: grub-core/osdep/unix/hostdisk.c: remove support for ancient glibc

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On 04.12.2014 23:21, Felix Janda wrote:
> On 28.11.2014, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 21.11.2014 22:00, Felix Janda wrote:
>>> Hello,
>>>
>>> the file grub-core/osdep/unix/hostdisk.c has special code for glibc<2.1,
>>> which gets also pulled in for other libcs (such as musl libc) and makes
>>> compilation fail for them because the code uses glibc internals. The
>>> easiest way to fix is just to remove the code as the below patch does.
>>>
>> Don't assume that nobody will try to compile with ancient glibc. If you
>> want to raise the supported version for glibc you need to add a
>> configure check for presence of long offsets to configure and fail it if
>> it's not available. It should also be documented in INSTALL.
> 
> Before messing around with configure.ac, I'd like know whether a patch
> (correctly) implementing what you suggest would be accepted.
> 
Yes, I'll accept removing this clause as long as failure on ./configure
time is clear in case of no long-files available.
> Another solution I would be happy with, is to change the conditions
> 
> # if !defined(__GLIBC__) || \
>         ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> 
> to
> 
> # ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 1)))
> 
> Felix
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]