[CRIU] [crtools-bot] A few cleanups to uts_ns

Cyrill Gorcunov gorcunov at openvz.org
Tue Jan 31 15:09:32 EST 2012


On Wed, Feb 01, 2012 at 12:01:57AM +0400, Kir Kolyshkin wrote:
> On 01/31/2012 11:35 PM, Cyrill Gorcunov wrote:
> >The commit is pushed to "master" and will appear on git://github.com/cyrillos/crtools.git
> >------>
> >commit ca099959343463088387df332107c2a6c5d47186
> >Author: Cyrill Gorcunov<gorcunov at openvz.org>
> >Date:   Tue Jan 31 23:29:24 2012 +0400
> >
> >     A few cleanups to uts_ns
> >
> >      - drop hardcoded numbers, use sizeof
> >      - drop unneeded local argument
> >
> >     Signed-off-by: Cyrill Gorcunov<gorcunov at openvz.org>
> >---
> >  uts_ns.c |    7 +++----
> >  1 files changed, 3 insertions(+), 4 deletions(-)
> >
> >diff --git a/uts_ns.c b/uts_ns.c
> >index af16805..04bfced 100644
> >--- a/uts_ns.c
> >+++ b/uts_ns.c
> >@@ -14,7 +14,7 @@ static int dump_uts_string(int fd, char *str)
> >
> >  	len = strlen(str);
> >  	ret = write_img(fd,&len);
> >-	if (ret == 0)
> >+	if (!ret)
> 
> This is a controversial change. Although (!ret) is kinda classic,
> (ret == 0) is a bit more readable for me than (!ret). The only
> problem with == version is sometimes it can be mistyped as =, but
> modern compilers do warn in this case. So I would left it as is (no
> need to change again though, just for the future).
> 

Kir, I would not touch it but there is a second function below,
so it was

static int dump_uts_string(int fd, char *str)
{
	int ret;
	u32 len;

	len = strlen(str);
	ret = write_img(fd, &len);
-->	if (ret == 0)
		ret = write_img_buf(fd, str, len);

	return ret;
}

int dump_uts_ns(int ns_pid, struct cr_fdset *fdset)
{
	int fd, ret;
	struct utsname ubuf;

	ret = switch_ns(ns_pid, CLONE_NEWUTS, "uts");
	if (ret < 0)
		return ret;

	ret = uname(&ubuf);
	if (ret < 0) {
		pr_perror("Error calling uname");
		return ret;
	}

	fd = fdset->fds[CR_FD_UTSNS];

	ret = dump_uts_string(fd, ubuf.nodename);
-->	if (!ret)
		ret = dump_uts_string(fd, ubuf.domainname);

	return ret;
}

it become unclear for me why two tests for zero are
written in different fashion. I think at least in close
code snippets the some common style should be used.

	Cyrill


More information about the CRIU mailing list