diff mbox series

[1/6] gdbstub: Return the fd from gdbserver_start

Message ID 20180531224911.23725-2-richard.henderson@linaro.org
State New
Headers show
Series linux-user: Reorg interp_prefix handling | expand

Commit Message

Richard Henderson May 31, 2018, 10:49 p.m. UTC
This will allow us to protect gdbserver_fd from the guest.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 gdbstub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.17.0

Comments

Philippe Mathieu-Daudé May 31, 2018, 11:15 p.m. UTC | #1
Hi Richard,

On 05/31/2018 07:49 PM, Richard Henderson wrote:
> This will allow us to protect gdbserver_fd from the guest.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  gdbstub.c | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)

> 

> diff --git a/gdbstub.c b/gdbstub.c

> index 6081e719c5..057d0d65c5 100644

> --- a/gdbstub.c

> +++ b/gdbstub.c

> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)

>  int gdbserver_start(int port)

>  {

>      gdbserver_fd = gdbserver_open(port);

> -    if (gdbserver_fd < 0)

> +    if (gdbserver_fd < 0) {

>          return -1;

> +    }

>      /* accept connections */

>      if (!gdb_accept()) {

>          close(gdbserver_fd);

>          gdbserver_fd = -1;

>          return -1;

>      }

> -    return 0;

> +    return gdbserver_fd;


I agree with your change, but what about !CONFIG_USER_ONLY?

It should be safe enough documenting the different behaviors in
include/exec/gdbstub.h.

>  }

>  

>  /* Disable gdb stub for child processes.  */

>
Richard Henderson June 1, 2018, 12:16 a.m. UTC | #2
On 05/31/2018 04:15 PM, Philippe Mathieu-Daudé wrote:
> Hi Richard,

> 

> On 05/31/2018 07:49 PM, Richard Henderson wrote:

>> This will allow us to protect gdbserver_fd from the guest.

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>  gdbstub.c | 5 +++--

>>  1 file changed, 3 insertions(+), 2 deletions(-)

>>

>> diff --git a/gdbstub.c b/gdbstub.c

>> index 6081e719c5..057d0d65c5 100644

>> --- a/gdbstub.c

>> +++ b/gdbstub.c

>> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)

>>  int gdbserver_start(int port)

>>  {

>>      gdbserver_fd = gdbserver_open(port);

>> -    if (gdbserver_fd < 0)

>> +    if (gdbserver_fd < 0) {

>>          return -1;

>> +    }

>>      /* accept connections */

>>      if (!gdb_accept()) {

>>          close(gdbserver_fd);

>>          gdbserver_fd = -1;

>>          return -1;

>>      }

>> -    return 0;

>> +    return gdbserver_fd;

> 

> I agree with your change, but what about !CONFIG_USER_ONLY?


It's still a non-negative number, so the success value is still the same.


r~
Peter Maydell June 1, 2018, 8:59 a.m. UTC | #3
On 31 May 2018 at 23:49, Richard Henderson <richard.henderson@linaro.org> wrote:
> This will allow us to protect gdbserver_fd from the guest.


Ha, I hadn't realised we already had an internal-to-QEMU filedescriptor :-)

thanks
-- PMM
Philippe Mathieu-Daudé June 1, 2018, 12:42 p.m. UTC | #4
On 05/31/2018 09:16 PM, Richard Henderson wrote:
> On 05/31/2018 04:15 PM, Philippe Mathieu-Daudé wrote:

>> Hi Richard,

>>

>> On 05/31/2018 07:49 PM, Richard Henderson wrote:

>>> This will allow us to protect gdbserver_fd from the guest.

>>>

>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>>> ---

>>>  gdbstub.c | 5 +++--

>>>  1 file changed, 3 insertions(+), 2 deletions(-)

>>>

>>> diff --git a/gdbstub.c b/gdbstub.c

>>> index 6081e719c5..057d0d65c5 100644

>>> --- a/gdbstub.c

>>> +++ b/gdbstub.c

>>> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)

>>>  int gdbserver_start(int port)

>>>  {

>>>      gdbserver_fd = gdbserver_open(port);

>>> -    if (gdbserver_fd < 0)

>>> +    if (gdbserver_fd < 0) {

>>>          return -1;

>>> +    }

>>>      /* accept connections */

>>>      if (!gdb_accept()) {

>>>          close(gdbserver_fd);

>>>          gdbserver_fd = -1;

>>>          return -1;

>>>      }

>>> -    return 0;

>>> +    return gdbserver_fd;

>>

>> I agree with your change, but what about !CONFIG_USER_ONLY?

> 

> It's still a non-negative number, so the success value is still the same.


Good point.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Richard Henderson June 1, 2018, 4:42 p.m. UTC | #5
On 06/01/2018 01:59 AM, Peter Maydell wrote:
> On 31 May 2018 at 23:49, Richard Henderson <richard.henderson@linaro.org> wrote:

>> This will allow us to protect gdbserver_fd from the guest.

> 

> Ha, I hadn't realised we already had an internal-to-QEMU filedescriptor :-)


Two, in fact, if you count qemu_logfile.  ;-)


r~
Laurent Vivier June 1, 2018, 8 p.m. UTC | #6
Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> This will allow us to protect gdbserver_fd from the guest.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Laurent Vivier <laurent@vivier.eu>
diff mbox series

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 6081e719c5..057d0d65c5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1890,15 +1890,16 @@  static int gdbserver_open(int port)
 int gdbserver_start(int port)
 {
     gdbserver_fd = gdbserver_open(port);
-    if (gdbserver_fd < 0)
+    if (gdbserver_fd < 0) {
         return -1;
+    }
     /* accept connections */
     if (!gdb_accept()) {
         close(gdbserver_fd);
         gdbserver_fd = -1;
         return -1;
     }
-    return 0;
+    return gdbserver_fd;
 }
 
 /* Disable gdb stub for child processes.  */