[CRIU] Re: [PATCH 3/4] pr_perror(): print error at the end of line

[Cyrill Gorcunov]

On Tue, Jan 31, 2012 at 03:15:07AM +0400, Kir Kolyshkin wrote:
...
> >
> >+void printk_strerrno(const char *format, ...)
> >+{
> >+	char buf[1024];
> >+	va_list params;
> >+	int npos;
> >+
> >+	va_start(params, format);
> >+	vsnprintf(buf, sizeof(buf), format, params);
> >+	va_end(params);
> >+
> >+	npos = strlen(buf) - 1;
> >+	if (buf[npos] == '\n')
> >+		buf[npos] = '\0';
> >+	else
> >+		npos = -1;
> >+
> >+	strncat(buf, ": ", sizeof(buf));
> >+	strncat(buf, strerror(errno), sizeof(buf));
> >+
> >+	if (npos != -1)
> >+		strncat(buf, "\n", sizeof(buf));
> >+
> >+	dprintf(get_logfd(), "%s", buf);
> >+}
> >+
> >
> 
> Well frankly speaking it looks pretty ugly to me:
> 1. Looks complex.
> 2. Artificial limit (1024) on resulting string size (could use
> vasprintf() here)
> 3. strncat() invocations are inefficient, they scan buf from the
> beginning till the end every time looking for \0, which you already
> did.

yes, but as I say it's not that important if this function is
called.

> 4. dprintf() looks like an overkill, what's wrong with write()?

just to use a pair of vdprintf

> 5. It doesn't handle the case with multiple \n at the end of format string.
> 
> Anyway, I came up with what I think is a slightly better version of
> the above. Still looks ugly to me though:
> 1. It still looks complex.
> 2. It still doesn't handle the case with multiple \n at the end.
> 
> If you and Pavel can tolerate it, let me know and I will redo my
> patch #3 with this one:
> 

Looks pretty good to me! Pavel?

	Cyrill