[PATCHv2,3/3] tty: Make unlock_pty() inline void

Submitted by Radostin Stoyanov on Nov. 17, 2018, 11:53 a.m.

Details

Message ID 20181117115329.16424-3-rstoyanov1@gmail.com
State Rejected
Series "Series without cover letter"
Headers show

Commit Message

Radostin Stoyanov Nov. 17, 2018, 11:53 a.m.
The return value of unlock_pty() is not needed because if the unlock
fails then futher operations on locked entry will fail as well.
unlock_pty() will only yield the error message.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/tty.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/tty.c b/criu/tty.c
index 38e1cab3..fe126f18 100644
--- a/criu/tty.c
+++ b/criu/tty.c
@@ -655,7 +655,7 @@  static int pty_open_ptmx_index(struct file_desc *d, struct tty_info *info, int f
 					open_tty_reg, d, path_from_reg(d));
 }
 
-static int unlock_pty(int fd)
+static void inline unlock_pty(int fd)
 {
 	const int lock = 0;
 
@@ -664,12 +664,8 @@  static int unlock_pty(int fd)
 	 * by kernel and we need to unlock it to be
 	 * able to connect slave peer.
 	 */
-	if (ioctl(fd, TIOCSPTLCK, &lock)) {
+	if (ioctl(fd, TIOCSPTLCK, &lock))
 		pr_err("Unable to unlock pty device via y%d\n", fd);
-		return -1;
-	}
-
-	return 0;
 }
 
 static int lock_pty(int fd)

Comments

Cyrill Gorcunov Nov. 17, 2018, 12:38 p.m.
On Sat, Nov 17, 2018 at 11:53:29AM +0000, Radostin Stoyanov wrote:
> The return value of unlock_pty() is not needed because if the unlock
> fails then futher operations on locked entry will fail as well.
> unlock_pty() will only yield the error message.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>

Thanks!
Andrei Vagin Jan. 9, 2019, 7:09 a.m.
On Sat, Nov 17, 2018 at 11:53:29AM +0000, Radostin Stoyanov wrote:
> The return value of unlock_pty() is not needed because if the unlock
> fails then futher operations on locked entry will fail as well.
> unlock_pty() will only yield the error message.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/tty.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/criu/tty.c b/criu/tty.c
> index 38e1cab3..fe126f18 100644
> --- a/criu/tty.c
> +++ b/criu/tty.c
> @@ -655,7 +655,7 @@ static int pty_open_ptmx_index(struct file_desc *d, struct tty_info *info, int f
>  					open_tty_reg, d, path_from_reg(d));
>  }
>  
> -static int unlock_pty(int fd)
> +static void inline unlock_pty(int fd)

What is the benefit?

An inline function can return int...

>  {
>  	const int lock = 0;
>  
> @@ -664,12 +664,8 @@ static int unlock_pty(int fd)
>  	 * by kernel and we need to unlock it to be
>  	 * able to connect slave peer.
>  	 */
> -	if (ioctl(fd, TIOCSPTLCK, &lock)) {
> +	if (ioctl(fd, TIOCSPTLCK, &lock))
>  		pr_err("Unable to unlock pty device via y%d\n", fd);
> -		return -1;
> -	}
> -
> -	return 0;
>  }
>  
>  static int lock_pty(int fd)
> -- 
> 2.17.2
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Radostin Stoyanov Jan. 9, 2019, 10:04 a.m.
On 09/01/2019 07:09, Andrei Vagin wrote:
> On Sat, Nov 17, 2018 at 11:53:29AM +0000, Radostin Stoyanov wrote:
>> The return value of unlock_pty() is not needed because if the unlock
>> fails then futher operations on locked entry will fail as well.
>> unlock_pty() will only yield the error message.
>>
>> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
>> ---
>>  criu/tty.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/criu/tty.c b/criu/tty.c
>> index 38e1cab3..fe126f18 100644
>> --- a/criu/tty.c
>> +++ b/criu/tty.c
>> @@ -655,7 +655,7 @@ static int pty_open_ptmx_index(struct file_desc *d, struct tty_info *info, int f
>>  					open_tty_reg, d, path_from_reg(d));
>>  }
>>  
>> -static int unlock_pty(int fd)
>> +static void inline unlock_pty(int fd)
> What is the benefit?
>
> An inline function can return int...
It is just that the return value of unlock_pty() is never being used.
Maybe I should change the commit message.
>
>>  {
>>  	const int lock = 0;
>>  
>> @@ -664,12 +664,8 @@ static int unlock_pty(int fd)
>>  	 * by kernel and we need to unlock it to be
>>  	 * able to connect slave peer.
>>  	 */
>> -	if (ioctl(fd, TIOCSPTLCK, &lock)) {
>> +	if (ioctl(fd, TIOCSPTLCK, &lock))
>>  		pr_err("Unable to unlock pty device via y%d\n", fd);
>> -		return -1;
>> -	}
>> -
>> -	return 0;
>>  }
>>  
>>  static int lock_pty(int fd)
>> -- 
>> 2.17.2
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
Andrei Vagin Jan. 10, 2019, 5:04 p.m.
On Wed, Jan 09, 2019 at 10:04:41AM +0000, Radostin Stoyanov wrote:
> On 09/01/2019 07:09, Andrei Vagin wrote:
> > On Sat, Nov 17, 2018 at 11:53:29AM +0000, Radostin Stoyanov wrote:
> >> The return value of unlock_pty() is not needed because if the unlock
> >> fails then futher operations on locked entry will fail as well.
> >> unlock_pty() will only yield the error message.
> >>
> >> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> >> ---
> >>  criu/tty.c | 8 ++------
> >>  1 file changed, 2 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/criu/tty.c b/criu/tty.c
> >> index 38e1cab3..fe126f18 100644
> >> --- a/criu/tty.c
> >> +++ b/criu/tty.c
> >> @@ -655,7 +655,7 @@ static int pty_open_ptmx_index(struct file_desc *d, struct tty_info *info, int f
> >>  					open_tty_reg, d, path_from_reg(d));
> >>  }
> >>  
> >> -static int unlock_pty(int fd)
> >> +static void inline unlock_pty(int fd)
> > What is the benefit?
> >
> > An inline function can return int...
> It is just that the return value of unlock_pty() is never being used.

or may be better to start halndling the return value

> Maybe I should change the commit message.
> >
> >>  {
> >>  	const int lock = 0;
> >>  
> >> @@ -664,12 +664,8 @@ static int unlock_pty(int fd)
> >>  	 * by kernel and we need to unlock it to be
> >>  	 * able to connect slave peer.
> >>  	 */
> >> -	if (ioctl(fd, TIOCSPTLCK, &lock)) {
> >> +	if (ioctl(fd, TIOCSPTLCK, &lock))
> >>  		pr_err("Unable to unlock pty device via y%d\n", fd);
> >> -		return -1;
> >> -	}
> >> -
> >> -	return 0;
> >>  }
> >>  
> >>  static int lock_pty(int fd)
> >> -- 
> >> 2.17.2
> >>
> >> _______________________________________________
> >> CRIU mailing list
> >> CRIU@openvz.org
> >> https://lists.openvz.org/mailman/listinfo/criu
>
Radostin Stoyanov Jan. 10, 2019, 5:14 p.m.
On 10/01/2019 17:04, Andrei Vagin wrote:
> On Wed, Jan 09, 2019 at 10:04:41AM +0000, Radostin Stoyanov wrote:
>> On 09/01/2019 07:09, Andrei Vagin wrote:
>>> On Sat, Nov 17, 2018 at 11:53:29AM +0000, Radostin Stoyanov wrote:
>>>> The return value of unlock_pty() is not needed because if the unlock
>>>> fails then futher operations on locked entry will fail as well.
>>>> unlock_pty() will only yield the error message.
>>>>
>>>> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
>>>> ---
>>>>  criu/tty.c | 8 ++------
>>>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/criu/tty.c b/criu/tty.c
>>>> index 38e1cab3..fe126f18 100644
>>>> --- a/criu/tty.c
>>>> +++ b/criu/tty.c
>>>> @@ -655,7 +655,7 @@ static int pty_open_ptmx_index(struct file_desc *d, struct tty_info *info, int f
>>>>  					open_tty_reg, d, path_from_reg(d));
>>>>  }
>>>>  
>>>> -static int unlock_pty(int fd)
>>>> +static void inline unlock_pty(int fd)
>>> What is the benefit?
>>>
>>> An inline function can return int...
>> It is just that the return value of unlock_pty() is never being used.
> or may be better to start halndling the return value
That was my first idea:
https://lists.openvz.org/pipermail/criu/2018-November/042929.html
https://lists.openvz.org/pipermail/criu/2018-November/042932.html
>
>> Maybe I should change the commit message.
>>>>  {
>>>>  	const int lock = 0;
>>>>  
>>>> @@ -664,12 +664,8 @@ static int unlock_pty(int fd)
>>>>  	 * by kernel and we need to unlock it to be
>>>>  	 * able to connect slave peer.
>>>>  	 */
>>>> -	if (ioctl(fd, TIOCSPTLCK, &lock)) {
>>>> +	if (ioctl(fd, TIOCSPTLCK, &lock))
>>>>  		pr_err("Unable to unlock pty device via y%d\n", fd);
>>>> -		return -1;
>>>> -	}
>>>> -
>>>> -	return 0;
>>>>  }
>>>>  
>>>>  static int lock_pty(int fd)
>>>> -- 
>>>> 2.17.2
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU@openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu