diff mbox series

[08/22] Implement target_set_brk function in bsd-mem.c instead of os-syscall.c

Message ID 20230819094806.14965-9-kariem.taha2.7@gmail.com
State New
Headers show
Series Implement the mmap system call for FreeBSD. | expand

Commit Message

Karim Taha Aug. 19, 2023, 9:47 a.m. UTC
From: Stacey Son <sson@FreeBSD.org>

Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ----
 2 files changed, 38 insertions(+), 4 deletions(-)

Comments

Warner Losh Aug. 20, 2023, 4:22 a.m. UTC | #1
On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ----
>  2 files changed, 38 insertions(+), 4 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

but see below


> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index e69de29bb2..6c123abf04 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -0,0 +1,38 @@
> +/*
> + *  memory management system conversion routines
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of 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.
> + *
> + *  This program 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 a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +
> +#include <sys/ipc.h>
> +#include <sys/shm.h>
> +
>

I'll let others comment if this is a good thing here or it needs to be
elsewhere.
I can't recall what the project's rules are for system headers on code that
runs on a limited subset of systems. For code that can run anywhere, I
know that the preference is to put this in osdep.h, but in this case I'm
unsure.

Warner


> +#include "qemu.h"
> +#include "qemu-bsd.h"
> +
> +struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];
> +
> +abi_ulong bsd_target_brk;
> +abi_ulong bsd_target_original_brk;
> +abi_ulong brk_page;
> +
> +void target_set_brk(abi_ulong new_brk)
> +{
> +
> +    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
> +    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
> +}
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 2920370ad2..c0a22eb746 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -59,10 +59,6 @@ safe_syscall3(ssize_t, writev, int, fd, const struct
> iovec *, iov, int, iovcnt);
>  safe_syscall4(ssize_t, pwritev, int, fd, const struct iovec *, iov, int,
> iovcnt,
>      off_t, offset);
>
> -void target_set_brk(abi_ulong new_brk)
> -{
> -}
> -
>  /*
>   * errno conversion.
>   */
> --
> 2.40.0
>
>
Richard Henderson Aug. 20, 2023, 2:12 p.m. UTC | #2
On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  4 ----
>   2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index e69de29bb2..6c123abf04 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -0,0 +1,38 @@
> +/*
> + *  memory management system conversion routines
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of 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.
> + *
> + *  This program 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 a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +
> +#include <sys/ipc.h>
> +#include <sys/shm.h>
> +
> +#include "qemu.h"
> +#include "qemu-bsd.h"
> +
> +struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];


This and the ipc/shm includes don't belong with this patch.


> +
> +abi_ulong bsd_target_brk;
> +abi_ulong bsd_target_original_brk;
> +abi_ulong brk_page;
> +
> +void target_set_brk(abi_ulong new_brk)
> +{
> +

Blank line.

> +    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
> +    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);

I encourage you to use TARGET_PAGE_ALIGN instead.
This will match changes made in linux-user during the 8.1 cycle.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
index e69de29bb2..6c123abf04 100644
--- a/bsd-user/bsd-mem.c
+++ b/bsd-user/bsd-mem.c
@@ -0,0 +1,38 @@ 
+/*
+ *  memory management system conversion routines
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of 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.
+ *
+ *  This program 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 a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+
+#include <sys/ipc.h>
+#include <sys/shm.h>
+
+#include "qemu.h"
+#include "qemu-bsd.h"
+
+struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];
+
+abi_ulong bsd_target_brk;
+abi_ulong bsd_target_original_brk;
+abi_ulong brk_page;
+
+void target_set_brk(abi_ulong new_brk)
+{
+
+    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
+    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
+}
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 2920370ad2..c0a22eb746 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -59,10 +59,6 @@  safe_syscall3(ssize_t, writev, int, fd, const struct iovec *, iov, int, iovcnt);
 safe_syscall4(ssize_t, pwritev, int, fd, const struct iovec *, iov, int, iovcnt,
     off_t, offset);
 
-void target_set_brk(abi_ulong new_brk)
-{
-}
-
 /*
  * errno conversion.
  */