Re: [PATCH v2 14/16] Add function sys_get_page_size to replace platform dependent sysconf (_SC_PAGESIZE)

[Mark Wielaard <mark@klomp.org>]

Hi,

On Sun, 2022-12-18 at 00:52 +0800, Yonggang Luo via Elfutils-devel
wrote:
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  lib/Makefile.am                |  2 +-
>  lib/crc32_file.c               |  2 +-
>  lib/system.c                   | 48 ++++++++++++++++++++++++++++++++++
>  lib/system.h                   |  3 +++
>  libdw/dwarf_begin_elf.c        |  2 +-
>  libdwelf/dwelf_strtab.c        |  2 +-
>  libdwfl/linux-kernel-modules.c |  2 +-
>  libdwfl/linux-proc-maps.c      |  2 +-
>  libelf/elf32_updatefile.c      |  2 +-
>  src/ar.c                       |  2 +-
>  src/ranlib.c                   |  2 +-
>  src/strings.c                  |  2 +-
>  12 files changed, 61 insertions(+), 10 deletions(-)
>  create mode 100644 lib/system.c
> 
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 42ddf5ae..7a50085b 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -35,7 +35,7 @@ noinst_LIBRARIES = libeu.a
>  
>  libeu_a_SOURCES = xasprintf.c xstrdup.c xstrndup.c xmalloc.c next_prime.c \
>  		  crc32.c crc32_file.c \
> -		  color.c error.c printversion.c
> +		  color.c error.c printversion.c system.c
>  
>  noinst_HEADERS = fixedsizehash.h libeu.h system.h dynamicsizehash.h list.h \
>  		 eu-config.h color.h printversion.h bpf.h \
> diff --git a/lib/crc32_file.c b/lib/crc32_file.c
> index 66833702..45e1cc52 100644
> --- a/lib/crc32_file.c
> +++ b/lib/crc32_file.c
> @@ -52,7 +52,7 @@ crc32_file (int fd, uint32_t *resp)
>        void *mapped = mmap (NULL, mapsize, PROT_READ, MAP_PRIVATE, fd, 0);
>        if (mapped == MAP_FAILED && errno == ENOMEM)
>  	{
> -	  const size_t pagesize = sysconf (_SC_PAGESIZE);
> +	  const size_t pagesize = sys_get_page_size();
>  	  mapsize = ((mapsize / 2) + pagesize - 1) & -pagesize;
>  	  while (mapsize >= pagesize
>  		 && (mapped = mmap (NULL, mapsize, PROT_READ, MAP_PRIVATE,
> diff --git a/lib/system.c b/lib/system.c
> new file mode 100644
> index 00000000..bd3831f5
> --- /dev/null
> +++ b/lib/system.c
> @@ -0,0 +1,48 @@
> +/* Definitions for system functions.
> +   Copyright (C) 2006-2011 Red Hat, Inc.
> +   Copyright (C) 2022 Mark J. Wielaard <mark@klomp.org>
> +   Copyright (C) 2022 Yonggang Luo <luoyonggang@gmail.com>
> +   This file is part of elfutils.
> +
> +   This file is free software; you can redistribute it and/or modify
> +   it under the terms of either
> +
> +     * the GNU Lesser General Public License as published by the Free
> +       Software Foundation; either version 3 of the License, or (at
> +       your option) any later version
> +
> +   or
> +
> +     * the GNU General Public License as published by the Free
> +       Software Foundation; either version 2 of the License, or (at
> +       your option) any later version
> +
> +   or both in parallel, as here.
> +
> +   elfutils is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received copies of the GNU General Public License and
> +   the GNU Lesser General Public License along with this program.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +
> +#include "system.h"
> +#if defined(_WIN32)
> +#include <windows.h>
> +#endif
> +
> +size_t
> +sys_get_page_size(void)
> +{
> +#ifdef _WIN32
> +  SYSTEM_INFO info;
> +  GetSystemInfo(&info);
> +  return info.dwPageSize;
> +#else
> +  return sysconf (_SC_PAGESIZE);
> +#endif
> +}

I am not against abstracting this, but can we use some configure check
to see how to get the pagesize? I don't really like these #ifdef
_WIN32.

Does your environment provide the getpagesize() function that comes
from SVr4, 4.4BSD, SUSv2 and was part of older POSIX?

Cheers,

Mark