Thank you.
2014-11-24 22:12 GMT+03:00 Kir Kolyshkin <[email protected]
<mailto:[email protected]>>:
Thanks, this looks better than the previous version.
Please see my comments inline.
Please send a new version to devel@, cc: kir@ (one email, not two
separate ones).
On 11/24/2014 09:46 AM, Andrey Pushkar wrote:
As pointed out in bug #2999, vzlist -o ip segfaults if the
output string (IP addresses list in this case)
exceeds the limit of 4096 characters. The issue is caused by
static memory allocation, which has been changed
to dynamic in this commit: a snprintf wrapper that implements
automatic buffer resizing has been introduced.
1 Please make the description fit in 80 columns.
2 Please briefly describe the test case(s) you were using to check
the patch.
https://bugzilla.openvz.org/2999
Reported-by: Michal Grzedzicki <[email protected] <mailto:[email protected]>>
Signed-off-by: Andrey Pushkar <[email protected]
<mailto:[email protected]>>
---
src/vzlist.c | 118
+++++++++++++++++++++++++++++++++-------------------------
1 files changed, 67 insertions(+), 51 deletions(-)
diff --git a/src/vzlist.c b/src/vzlist.c
index 0b66f75..38bb7ae 100644
--- a/src/vzlist.c
+++ b/src/vzlist.c
@@ -21,6 +21,7 @@
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
+#include <stdarg.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/socket.h>
@@ -51,9 +52,10 @@
static struct Cveinfo *veinfo = NULL;
static int n_veinfo = 0;
-static char g_buf[4096] = "";
-static char *p_buf = g_buf;
-static char *e_buf = g_buf + sizeof(g_buf) - 1;
+static char *g_buf = NULL;
+static long g_buf_size = 4096;
+static char *p_buf = NULL;
+static char *e_buf = NULL;
static char *host_pattern = NULL;
static char *name_pattern = NULL;
static char *desc_pattern = NULL;
@@ -90,6 +92,9 @@ static inline int get_run_ve(int update)
#define get_run_ve get_run_ve_proc
#endif
+static int x_snprintf(const char *format, ...);
1 This is no longer snprintf-like function, please rename. You can
call it xprintf or something.
2 Please use __attribute__ (__format__ ( __printf__, 1, 2))) here,
so C compiler can
check the arguments. See
https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
+static void *x_realloc(void *ptr, int size);
+
I think you don't need this prototype here.
/* Print functions */
static void print_json_str(const char *s)
{
@@ -110,8 +115,7 @@ static void print_ ## funcname(struct
Cveinfo *p, int index) \
\
if (p->fieldname != NULL) \
str = p->fieldname; \
- r = snprintf(p_buf, e_buf - p_buf, \
- "%" #length "s", str); \
+ r = x_snprintf("%" #length "s", str); \
if (!is_last_field) \
r = abs(length); \
p_buf += r; \
@@ -177,7 +181,7 @@ static void print_strlist(char *list)
if ((ch = strchr(str, ' ')) != NULL)
*ch = 0;
}
- r = snprintf(p_buf, e_buf - p_buf, "%-15s", str);
+ r = x_snprintf("%-15s", str);
if (!is_last_field)
r = 15;
p_buf += r;
@@ -203,8 +207,7 @@ static void print_veid(struct Cveinfo *p,
int index)
if (fmt_json)
printf("%d", p->veid);
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%10d", p->veid);
+ p_buf += x_snprintf("%10d", p->veid);
}
static void print_smart_name(struct Cveinfo *p, int index)
@@ -220,8 +223,7 @@ static void print_status(struct Cveinfo
*p, int index)
if (fmt_json)
print_json_str(ve_status[p->status]);
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%-9s", ve_status[p->status]);
+ p_buf += x_snprintf("%-9s", ve_status[p->status]);
}
static void print_laverage(struct Cveinfo *p, int index)
@@ -230,8 +232,7 @@ static void print_laverage(struct Cveinfo
*p, int index)
if (fmt_json)
printf("null");
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%14s", "-");
+ p_buf += x_snprintf("%14s", "-");
}
else {
if (fmt_json)
@@ -240,8 +241,7 @@ static void print_laverage(struct Cveinfo
*p, int index)
p->cpustat->la[1],
p->cpustat->la[2]);
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%1.2f/%1.2f/%1.2f",
+ p_buf += x_snprintf("%1.2f/%1.2f/%1.2f",
p->cpustat->la[0],
p->cpustat->la[1],
p->cpustat->la[2]);
@@ -257,8 +257,7 @@ static void print_uptime(struct Cveinfo
*p, int index)
}
if (p->cpustat == NULL)
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%15s", "-");
+ p_buf += x_snprintf("%15s", "-");
else
{
unsigned int days, hours, min, secs;
@@ -272,8 +271,7 @@ static void print_uptime(struct Cveinfo
*p, int index)
(60ull * min + 60ull * 60 *
hours +
60ull * 60 * 24 * days));
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%.3dd%.2dh:%.2dm:%.2ds",
+ p_buf += x_snprintf("%.3dd%.2dh:%.2dm:%.2ds",
days, hours, min, secs);
}
}
@@ -285,15 +283,13 @@ static void print_cpu ## name(struct
Cveinfo *p, int index) \
if (fmt_json) \
printf("null"); \
else \
- p_buf += snprintf(p_buf, e_buf - p_buf, \
- "%7s", "-"); \
+ p_buf += x_snprintf("%7s", "-");
\
} \
else { \
if (fmt_json) \
printf("%lu", p->cpu->name[index]); \
else \
- p_buf += snprintf(p_buf, e_buf - p_buf, \
- "%7lu", p->cpu->name[index]); \
+ p_buf += x_snprintf("%7lu",
p->cpu->name[index]); \
} \
}
@@ -311,11 +307,9 @@ static void print_io ## name(struct
Cveinfo *p, int index) \
} \
\
if (i < min) \
- p_buf += snprintf(p_buf, e_buf - p_buf, \
- fmt "s", "-"); \
+ p_buf += x_snprintf(fmt "s", "-");
\
else \
- p_buf += snprintf(p_buf, e_buf - p_buf, \
- fmt "d", i); \
+ p_buf += x_snprintf(fmt "d", i);
\
}
PRINT_IO(prio, "%3", 0)
@@ -328,8 +322,7 @@ static void print_bool(const char *fmt,
int val)
if (fmt_json)
printf(val ? "true" : "false");
else
- p_buf += snprintf(p_buf, e_buf-p_buf,
- fmt, val ? "yes" : "no");
+ p_buf += x_snprintf(fmt, val ? "yes" : "no");
}
static void print_onboot(struct Cveinfo *p, int index)
@@ -341,7 +334,7 @@ static void print_onboot(struct Cveinfo
*p, int index)
if (fmt_json)
printf("null");
else
- p_buf += snprintf(p_buf, e_buf-p_buf, "%6s", "-");
+ p_buf += x_snprintf("%6s", "-");
}
static void print_bootorder(struct Cveinfo *p, int index)
@@ -350,15 +343,13 @@ static void print_bootorder(struct
Cveinfo *p, int index)
if (fmt_json)
printf("null");
else
- p_buf += snprintf(p_buf, e_buf-p_buf,
- "%10s", "-");
+ p_buf += x_snprintf("%10s", "-");
}
else {
if (fmt_json)
printf("%lu", p->bootorder[index]);
else
- p_buf += snprintf(p_buf, e_buf-p_buf,
- "%10lu",
p->bootorder[index]);
+ p_buf += x_snprintf("%10lu",
p->bootorder[index]);
}
}
@@ -370,11 +361,9 @@ static void print_cpunum(struct Cveinfo
*p, int index)
}
if (p->cpunum <= 0)
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%5s", "-");
+ p_buf += x_snprintf("%5s", "-");
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%5d", p->cpunum);
+ p_buf += x_snprintf("%5d", p->cpunum);
}
static const char *layout2str(int layout)
@@ -394,8 +383,7 @@ static void print_layout(struct Cveinfo
*p, int index)
if (fmt_json)
print_json_str(layout2str(p->layout));
else
- p_buf += snprintf(p_buf, e_buf - p_buf,
- "%-6s", layout2str(p->layout));
+ p_buf += x_snprintf("%-6s",
layout2str(p->layout));
}
static void print_features(struct Cveinfo *p, int index)
@@ -409,7 +397,7 @@ static void print_features(struct Cveinfo
*p, int index)
features_mask2str(p->features_mask, p->features_known,
",", str, sizeof(str));
- r = snprintf(p_buf, e_buf - p_buf, "%-15s", str);
+ r = x_snprintf("%-15s", str);
if (!is_last_field)
r = 15;
p_buf += r;
@@ -440,9 +428,9 @@ static void print_ubc(struct Cveinfo *p,
size_t res_off, int index)
}
if (!res || (!running && (index == 0 || index == 1 ||
index == 4)))
- p_buf += snprintf(p_buf, e_buf - p_buf,
"%10s", "-");
+ p_buf += x_snprintf("%10s", "-");
else
- p_buf += snprintf(p_buf, e_buf - p_buf, "%10lu",
+ p_buf += x_snprintf("%10lu",
res[index]);
}
@@ -493,10 +481,9 @@ static void print_dq(struct Cveinfo *p,
size_t res_off, int index)
}
if (!res || (index == 0 && res[index] == 0))
- p_buf += snprintf(p_buf, e_buf - p_buf,
"%10s", "-");
+ p_buf += x_snprintf("%10s", "-");
else
- p_buf += snprintf(p_buf, e_buf - p_buf, "%10lu",
- res[index]);
+ p_buf += x_snprintf("%10lu", res[index]);
}
#define PRINT_DQ(name) \
@@ -517,10 +504,9 @@ static void print_vm_overcommit(struct
Cveinfo *p, int index)
}
if (p->vm_overcommit == 0)
- p_buf += snprintf(p_buf, e_buf - p_buf, "%6s",
"-");
+ p_buf += x_snprintf("%6s", "-");
else
- p_buf += snprintf(p_buf, e_buf - p_buf, "%6g",
- p->vm_overcommit);
+ p_buf += x_snprintf("%6g", p->vm_overcommit);
}
/* Sort functions */
@@ -790,6 +776,32 @@ static void *x_realloc(void *ptr, int size)
return tmp;
}
+static int x_snprintf(const char *format, ...)
+{
+ va_list arglist, tmp_arglist;
+ va_start(arglist, format);
+ va_start(tmp_arglist, format);
+
+ int write_len = vsnprintf(p_buf, e_buf - p_buf,
format, tmp_arglist);
+
+ write_len++; // vsnprintf doesn't count the last \0
+
+ if (e_buf - p_buf <= write_len)
+ {
+ g_buf_size = write_len >= g_buf_size * 2 ?
write_len : g_buf_size * 2;
+ char *tmp = (char *)x_realloc(g_buf,
sizeof(char) * g_buf_size);
+
+ p_buf = tmp + (p_buf - g_buf);
+ g_buf = tmp;
+ e_buf = g_buf + g_buf_size - 1;
+ }
+
+ int ret = vsnprintf(p_buf, e_buf - p_buf, format,
arglist);
1 Don't declare variables in the middle of the function
2 You don't need to print two times in fact -- only if you need to
resize the buffer.
+ va_end(arglist);
+ va_end(tmp_arglist);
+ return ret;
+}
+
static void usage()
{
printf(
@@ -843,8 +855,7 @@ static void print_hdr()
for (p = g_field_order; p != NULL; p = p->next) {
f = p->order;
- p_buf += snprintf(p_buf, e_buf - p_buf,
- field_names[f].hdr_fmt,
field_names[f].hdr);
+ p_buf += x_snprintf(field_names[f].hdr_fmt,
field_names[f].hdr);
if (p_buf >= e_buf)
break;
if (p->next != NULL)
@@ -1673,7 +1684,7 @@ static int get_ves_cpu()
}
if (id && !veid) {
rate = rint((double)rate * 100 / 1024);
- weight = rint((double)MAXCPUUNITS /
weight);
+ weight = MAXCPUUNITS / weight;
Uh oh. Please rebase your stuff properly.
update_cpu(id, rate, weight);
}
}
@@ -1911,6 +1922,10 @@ int main(int argc, char **argv)
char *ep, *p;
int veid, c;
+ g_buf = (char *)malloc(sizeof(char) * g_buf_size);
+ p_buf = g_buf;
+ e_buf = g_buf + g_buf_size - 1;
+
while (1) {
int option_index = -1;
c = getopt_long(argc, argv, "HtSanjN:h:d:o:s:Le1",
@@ -2015,6 +2030,7 @@ int main(int argc, char **argv)
free(name_pattern);
free(desc_pattern);
free(f_order);
+ free(g_buf);
return 0;
}
