<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<div class="moz-cite-prefix">On 11/24/2014 11:20 AM, Андрей Пушкарь
wrote:<br>
</div>
<blockquote
cite="mid:CAMtvS2YunaEn1FT+3DmZi2L+zRuYWHm_bSjVU3FzAvmsCJpC7Q@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="ltr">Thank you. Few more questions:
<div>1) declaring variables almost everywhere we want is a part
of C99 standard, as I understand. Could you please explain
what is wrong with this practice?</div>
</div>
</blockquote>
<br>
It's just that the current code doesn't do it, and I don't like to
change that now.<br>
<br>
<blockquote
cite="mid:CAMtvS2YunaEn1FT+3DmZi2L+zRuYWHm_bSjVU3FzAvmsCJpC7Q@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>2) I'm sorry, but I didn't get what you mean by the last
line of your letter.</div>
</div>
</blockquote>
<br>
Your code accidentally reverted my recent commit. You can<br>
use git pull --rebase to avoid that, and you should always check
your<br>
patch before sending to make sure there are no changes you<br>
don't intend to have.<br>
<br>
<blockquote
cite="mid:CAMtvS2YunaEn1FT+3DmZi2L+zRuYWHm_bSjVU3FzAvmsCJpC7Q@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thank you.</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">2014-11-24 22:12 GMT+03:00 Kir
Kolyshkin <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:kir@openvz.org" target="_blank">kir@openvz.org</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks,
this looks better than the previous version.<br>
<br>
Please see my comments inline.<br>
<br>
Please send a new version to devel@, cc: kir@ (one email,
not two separate ones).<span class=""><br>
<br>
<br>
On 11/24/2014 09:46 AM, Andrey Pushkar wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
As pointed out in bug #2999, vzlist -o ip segfaults if
the output string (IP addresses list in this case)<br>
exceeds the limit of 4096 characters. The issue is
caused by static memory allocation, which has been
changed<br>
to dynamic in this commit: a snprintf wrapper that
implements automatic buffer resizing has been
introduced.<br>
</blockquote>
<br>
</span>
1 Please make the description fit in 80 columns.<br>
<br>
2 Please briefly describe the test case(s) you were using to
check the patch.
<div>
<div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a moz-do-not-send="true"
href="https://bugzilla.openvz.org/2999"
target="_blank">https://bugzilla.openvz.org/2999</a><br>
<br>
Reported-by: Michal Grzedzicki <<a
moz-do-not-send="true" href="mailto:lazy@iq.pl"
target="_blank">lazy@iq.pl</a>><br>
Signed-off-by: Andrey Pushkar <<a
moz-do-not-send="true"
href="mailto:andrey.s.pushkar@gmail.com"
target="_blank">andrey.s.pushkar@gmail.com</a>><br>
---<br>
src/vzlist.c | 118 +++++++++++++++++++++++++++++++++-------------------------<br>
1 files changed, 67 insertions(+), 51 deletions(-)<br>
<br>
diff --git a/src/vzlist.c b/src/vzlist.c<br>
index 0b66f75..38bb7ae 100644<br>
--- a/src/vzlist.c<br>
+++ b/src/vzlist.c<br>
@@ -21,6 +21,7 @@<br>
#include <stdlib.h><br>
#include <ctype.h><br>
#include <string.h><br>
+#include <stdarg.h><br>
#include <sys/types.h><br>
#include <sys/stat.h><br>
#include <sys/socket.h><br>
@@ -51,9 +52,10 @@<br>
static struct Cveinfo *veinfo = NULL;<br>
static int n_veinfo = 0;<br>
-static char g_buf[4096] = "";<br>
-static char *p_buf = g_buf;<br>
-static char *e_buf = g_buf + sizeof(g_buf) - 1;<br>
+static char *g_buf = NULL;<br>
+static long g_buf_size = 4096;<br>
+static char *p_buf = NULL;<br>
+static char *e_buf = NULL;<br>
static char *host_pattern = NULL;<br>
static char *name_pattern = NULL;<br>
static char *desc_pattern = NULL;<br>
@@ -90,6 +92,9 @@ static inline int get_run_ve(int
update)<br>
#define get_run_ve get_run_ve_proc<br>
#endif<br>
+static int x_snprintf(const char *format, ...);<br>
</blockquote>
<br>
</div>
</div>
1 This is no longer snprintf-like function, please rename.
You can call it xprintf or something.<br>
<br>
2 Please use __attribute__ (__format__ ( __printf__, 1, 2)))
here, so C compiler can<br>
check the arguments. See <a moz-do-not-send="true"
href="https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html"
target="_blank">https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html</a><span
class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
+static void *x_realloc(void *ptr, int size);<br>
+<br>
</blockquote>
<br>
</span>
I think you don't need this prototype here.
<div>
<div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
/* Print functions */<br>
static void print_json_str(const char *s)<br>
{<br>
@@ -110,8 +115,7 @@ static void print_ ##
funcname(struct Cveinfo *p, int index) \<br>
\<br>
if (p->fieldname != NULL)
\<br>
str = p->fieldname;
\<br>
- r = snprintf(p_buf, e_buf - p_buf,
\<br>
- "%" #length "s", str);
\<br>
+ r = x_snprintf("%" #length "s", str);
\<br>
if (!is_last_field)
\<br>
r = abs(length);
\<br>
p_buf += r;
\<br>
@@ -177,7 +181,7 @@ static void print_strlist(char
*list)<br>
if ((ch = strchr(str, ' ')) != NULL)<br>
*ch = 0;<br>
}<br>
- r = snprintf(p_buf, e_buf - p_buf, "%-15s",
str);<br>
+ r = x_snprintf("%-15s", str);<br>
if (!is_last_field)<br>
r = 15;<br>
p_buf += r;<br>
@@ -203,8 +207,7 @@ static void print_veid(struct
Cveinfo *p, int index)<br>
if (fmt_json)<br>
printf("%d", p->veid);<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%10d", p->veid);<br>
+ p_buf += x_snprintf("%10d",
p->veid);<br>
}<br>
static void print_smart_name(struct Cveinfo *p,
int index)<br>
@@ -220,8 +223,7 @@ static void print_status(struct
Cveinfo *p, int index)<br>
if (fmt_json)<br>
print_json_str(ve_status[p->status]);<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%-9s",
ve_status[p->status]);<br>
+ p_buf += x_snprintf("%-9s",
ve_status[p->status]);<br>
}<br>
static void print_laverage(struct Cveinfo *p, int
index)<br>
@@ -230,8 +232,7 @@ static void print_laverage(struct
Cveinfo *p, int index)<br>
if (fmt_json)<br>
printf("null");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf
- p_buf,<br>
- "%14s", "-");<br>
+ p_buf += x_snprintf("%14s",
"-");<br>
}<br>
else {<br>
if (fmt_json)<br>
@@ -240,8 +241,7 @@ static void print_laverage(struct
Cveinfo *p, int index)<br>
p->cpustat->la[1],<br>
p->cpustat->la[2]);<br>
else<br>
- p_buf += snprintf(p_buf, e_buf
- p_buf,<br>
-
"%1.2f/%1.2f/%1.2f",<br>
+ p_buf +=
x_snprintf("%1.2f/%1.2f/%1.2f",<br>
p->cpustat->la[0],<br>
p->cpustat->la[1],<br>
p->cpustat->la[2]);<br>
@@ -257,8 +257,7 @@ static void print_uptime(struct
Cveinfo *p, int index)<br>
}<br>
if (p->cpustat == NULL)<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%15s", "-");<br>
+ p_buf += x_snprintf("%15s", "-");<br>
else<br>
{<br>
unsigned int days, hours, min, secs;<br>
@@ -272,8 +271,7 @@ static void print_uptime(struct
Cveinfo *p, int index)<br>
(60ull * min + 60ull *
60 * hours +<br>
60ull * 60 * 24 *
days));<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
-
"%.3dd%.2dh:%.2dm:%.2ds",<br>
+ p_buf +=
x_snprintf("%.3dd%.2dh:%.2dm:%.2ds",<br>
days, hours, min,
secs);<br>
}<br>
}<br>
@@ -285,15 +283,13 @@ static void print_cpu ##
name(struct Cveinfo *p, int index) \<br>
if (fmt_json)
\<br>
printf("null");
\<br>
else
\<br>
- p_buf += snprintf(p_buf, e_buf
- p_buf, \<br>
- "%7s", "-");
\<br>
+ p_buf += x_snprintf("%7s",
"-"); \<br>
}
\<br>
else {
\<br>
if (fmt_json)
\<br>
printf("%lu",
p->cpu->name[index]); \<br>
else
\<br>
- p_buf += snprintf(p_buf, e_buf
- p_buf, \<br>
- "%7lu",
p->cpu->name[index]); \<br>
+ p_buf += x_snprintf("%7lu",
p->cpu->name[index]); \<br>
}
\<br>
}<br>
@@ -311,11 +307,9 @@ static void print_io ##
name(struct Cveinfo *p, int index) \<br>
}
\<br>
\<br>
if (i < min)
\<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, \<br>
- fmt "s", "-");
\<br>
+ p_buf += x_snprintf(fmt "s", "-");
\<br>
else
\<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, \<br>
- fmt "d", i);
\<br>
+ p_buf += x_snprintf(fmt "d", i);
\<br>
}<br>
PRINT_IO(prio, "%3", 0)<br>
@@ -328,8 +322,7 @@ static void print_bool(const char
*fmt, int val)<br>
if (fmt_json)<br>
printf(val ? "true" : "false");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf-p_buf,<br>
- fmt, val ? "yes" :
"no");<br>
+ p_buf += x_snprintf(fmt, val ? "yes" :
"no");<br>
}<br>
static void print_onboot(struct Cveinfo *p, int
index)<br>
@@ -341,7 +334,7 @@ static void print_onboot(struct
Cveinfo *p, int index)<br>
if (fmt_json)<br>
printf("null");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf-p_buf,
"%6s", "-");<br>
+ p_buf += x_snprintf("%6s", "-");<br>
}<br>
static void print_bootorder(struct Cveinfo *p, int
index)<br>
@@ -350,15 +343,13 @@ static void
print_bootorder(struct Cveinfo *p, int index)<br>
if (fmt_json)<br>
printf("null");<br>
else<br>
- p_buf += snprintf(p_buf,
e_buf-p_buf,<br>
- "%10s", "-");<br>
+ p_buf += x_snprintf("%10s",
"-");<br>
}<br>
else {<br>
if (fmt_json)<br>
printf("%lu",
p->bootorder[index]);<br>
else<br>
- p_buf += snprintf(p_buf,
e_buf-p_buf,<br>
- "%10lu",
p->bootorder[index]);<br>
+ p_buf += x_snprintf("%10lu",
p->bootorder[index]);<br>
}<br>
}<br>
@@ -370,11 +361,9 @@ static void print_cpunum(struct
Cveinfo *p, int index)<br>
}<br>
if (p->cpunum <= 0)<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%5s", "-");<br>
+ p_buf += x_snprintf("%5s", "-");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%5d", p->cpunum);<br>
+ p_buf += x_snprintf("%5d",
p->cpunum);<br>
}<br>
static const char *layout2str(int layout)<br>
@@ -394,8 +383,7 @@ static void print_layout(struct
Cveinfo *p, int index)<br>
if (fmt_json)<br>
print_json_str(layout2str(p->layout));<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf,<br>
- "%-6s",
layout2str(p->layout));<br>
+ p_buf += x_snprintf("%-6s",
layout2str(p->layout));<br>
}<br>
static void print_features(struct Cveinfo *p, int
index)<br>
@@ -409,7 +397,7 @@ static void print_features(struct
Cveinfo *p, int index)<br>
features_mask2str(p->features_mask,
p->features_known,<br>
",", str, sizeof(str));<br>
- r = snprintf(p_buf, e_buf - p_buf, "%-15s",
str);<br>
+ r = x_snprintf("%-15s", str);<br>
if (!is_last_field)<br>
r = 15;<br>
p_buf += r;<br>
@@ -440,9 +428,9 @@ static void print_ubc(struct
Cveinfo *p, size_t res_off, int index)<br>
}<br>
if (!res || (!running && (index == 0
|| index == 1 || index == 4)))<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%10s", "-");<br>
+ p_buf += x_snprintf("%10s", "-");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%10lu",<br>
+ p_buf += x_snprintf("%10lu",<br>
res[index]);<br>
}<br>
@@ -493,10 +481,9 @@ static void print_dq(struct
Cveinfo *p, size_t res_off, int index)<br>
}<br>
if (!res || (index == 0 && res[index]
== 0))<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%10s", "-");<br>
+ p_buf += x_snprintf("%10s", "-");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%10lu",<br>
- res[index]);<br>
+ p_buf += x_snprintf("%10lu",
res[index]);<br>
}<br>
#define PRINT_DQ(name)
\<br>
@@ -517,10 +504,9 @@ static void
print_vm_overcommit(struct Cveinfo *p, int index)<br>
}<br>
if (p->vm_overcommit == 0)<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%6s", "-");<br>
+ p_buf += x_snprintf("%6s", "-");<br>
else<br>
- p_buf += snprintf(p_buf, e_buf -
p_buf, "%6g",<br>
- p->vm_overcommit);<br>
+ p_buf += x_snprintf("%6g",
p->vm_overcommit);<br>
}<br>
/* Sort functions */<br>
@@ -790,6 +776,32 @@ static void *x_realloc(void *ptr,
int size)<br>
return tmp;<br>
}<br>
+static int x_snprintf(const char *format, ...)<br>
+{<br>
+ va_list arglist, tmp_arglist;<br>
+ va_start(arglist, format);<br>
+ va_start(tmp_arglist, format);<br>
+ <br>
+ int write_len = vsnprintf(p_buf, e_buf -
p_buf, format, tmp_arglist);<br>
+<br>
+ write_len++; // vsnprintf doesn't count the
last \0<br>
+<br>
+ if (e_buf - p_buf <= write_len)<br>
+ {<br>
+ g_buf_size = write_len >=
g_buf_size * 2 ? write_len : g_buf_size * 2;<br>
+ char *tmp = (char *)x_realloc(g_buf,
sizeof(char) * g_buf_size);<br>
+<br>
+ p_buf = tmp + (p_buf - g_buf);<br>
+ g_buf = tmp;<br>
+ e_buf = g_buf + g_buf_size - 1;<br>
+ }<br>
+<br>
+ int ret = vsnprintf(p_buf, e_buf - p_buf,
format, arglist);<br>
</blockquote>
<br>
</div>
</div>
1 Don't declare variables in the middle of the function<br>
<br>
2 You don't need to print two times in fact -- only if you
need to resize the buffer.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
+ va_end(arglist);<br>
+ va_end(tmp_arglist);<br>
+ return ret;<br>
+}<br>
+<br>
static void usage()<br>
{<br>
printf(<br>
@@ -843,8 +855,7 @@ static void print_hdr()<br>
for (p = g_field_order; p != NULL; p =
p->next) {<br>
f = p->order;<br>
- p_buf += snprintf(p_buf, e_buf - p_buf,<br>
- field_names[f].hdr_fmt,
field_names[f].hdr);<br>
+ p_buf += x_snprintf(field_names[f].hdr_fmt,
field_names[f].hdr);<br>
if (p_buf >= e_buf)<br>
break;<br>
if (p->next != NULL)<br>
@@ -1673,7 +1684,7 @@ static int get_ves_cpu()<br>
}<br>
if (id && !veid) {<br>
rate = rint((double)rate * 100 /
1024);<br>
- weight =
rint((double)MAXCPUUNITS / weight);<br>
+ weight = MAXCPUUNITS / weight;<br>
</blockquote>
<br>
</span>
Uh oh. Please rebase your stuff properly.
<div class="HOEnZb">
<div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
update_cpu(id, rate, weight);<br>
}<br>
}<br>
@@ -1911,6 +1922,10 @@ int main(int argc, char **argv)<br>
char *ep, *p;<br>
int veid, c;<br>
+ g_buf = (char *)malloc(sizeof(char) *
g_buf_size);<br>
+ p_buf = g_buf;<br>
+ e_buf = g_buf + g_buf_size - 1;<br>
+<br>
while (1) {<br>
int option_index = -1;<br>
c = getopt_long(argc, argv,
"HtSanjN:h:d:o:s:Le1",<br>
@@ -2015,6 +2030,7 @@ int main(int argc, char **argv)<br>
free(name_pattern);<br>
free(desc_pattern);<br>
free(f_order);<br>
+ free(g_buf);<br>
return 0;<br>
}<br>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>