diff mbox series

[V2] util: Remove redundant checks in the openpty()

Message ID 5F9FE5B8.1030803@huawei.com
State New
Headers show
Series [V2] util: Remove redundant checks in the openpty() | expand

Commit Message

Alex Chen Nov. 2, 2020, 10:55 a.m. UTC
As we can see from the following function call stack, amaster and aslave
can not be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
In addition, according to the API specification for openpty():
https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html,
the arguments name, termp and winp can all be NULL, but arguments amaster or aslave
can not be NULL.
Finally, amaster and aslave has been dereferenced at the beginning of the openpty().
So the checks on amaster and aslave in the openpty() are redundant. Remove them.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 util/qemu-openpty.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Alex Chen Sept. 14, 2021, 7:43 a.m. UTC | #1
Hi all,

This patch has been reviewed by Peter. who can help merge it?

Thanks,
Alex

On 2020/11/2 18:55, AlexChen wrote:
> As we can see from the following function call stack, amaster and aslave
> can not be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
> In addition, according to the API specification for openpty():
> https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html,
> the arguments name, termp and winp can all be NULL, but arguments amaster or aslave
> can not be NULL.
> Finally, amaster and aslave has been dereferenced at the beginning of the openpty().
> So the checks on amaster and aslave in the openpty() are redundant. Remove them.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Alex Chen <alex.chen@huawei.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  util/qemu-openpty.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index eb17f5b0bc..427f43a769 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name,
>              (termp != NULL && tcgetattr(sfd, termp) < 0))
>                  goto err;
> 
> -        if (amaster)
> -                *amaster = mfd;
> -        if (aslave)
> -                *aslave = sfd;
> +        *amaster = mfd;
> +        *aslave = sfd;
> +
>          if (winp)
>                  ioctl(sfd, TIOCSWINSZ, winp);
>
Laurent Vivier Sept. 15, 2021, 12:44 p.m. UTC | #2
Le 14/09/2021 à 09:43, Alex Chen a écrit :
> Hi all,
> 
> This patch has been reviewed by Peter. who can help merge it?
> 
> Thanks,
> Alex
> 
> On 2020/11/2 18:55, AlexChen wrote:
>> As we can see from the following function call stack, amaster and aslave
>> can not be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
>> In addition, according to the API specification for openpty():
>> https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html,
>> the arguments name, termp and winp can all be NULL, but arguments amaster or aslave
>> can not be NULL.
>> Finally, amaster and aslave has been dereferenced at the beginning of the openpty().
>> So the checks on amaster and aslave in the openpty() are redundant. Remove them.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  util/qemu-openpty.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
>> index eb17f5b0bc..427f43a769 100644
>> --- a/util/qemu-openpty.c
>> +++ b/util/qemu-openpty.c
>> @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name,
>>              (termp != NULL && tcgetattr(sfd, termp) < 0))
>>                  goto err;
>>
>> -        if (amaster)
>> -                *amaster = mfd;
>> -        if (aslave)
>> -                *aslave = sfd;
>> +        *amaster = mfd;
>> +        *aslave = sfd;
>> +
>>          if (winp)
>>                  ioctl(sfd, TIOCSWINSZ, winp);
>>
> 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent
diff mbox series

Patch

diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
index eb17f5b0bc..427f43a769 100644
--- a/util/qemu-openpty.c
+++ b/util/qemu-openpty.c
@@ -80,10 +80,9 @@  static int openpty(int *amaster, int *aslave, char *name,
             (termp != NULL && tcgetattr(sfd, termp) < 0))
                 goto err;

-        if (amaster)
-                *amaster = mfd;
-        if (aslave)
-                *aslave = sfd;
+        *amaster = mfd;
+        *aslave = sfd;
+
         if (winp)
                 ioctl(sfd, TIOCSWINSZ, winp);