trace: Put trace buffer on the stack instead of in the handle.

This makes more sense, and makes the code slightly closer to being
thread safe (although it's still NOT thread safe).
This commit is contained in:
Richard W.M. Jones
2013-03-04 09:53:10 +00:00
parent 8e2b920fe6
commit 66351f7494
3 changed files with 76 additions and 78 deletions

View File

@@ -1067,9 +1067,9 @@ and generate_client_actions hash () =
pr "\n"
);
pr " trace_fp = guestfs___trace_open (g);\n";
pr " guestfs___trace_open (&trace_buffer);\n";
pr " fprintf (trace_fp, \"%%s\", \"%s\");\n" name;
pr " fprintf (trace_buffer.fp, \"%%s\", \"%s\");\n" name;
(* Required arguments. *)
List.iter (
@@ -1082,33 +1082,35 @@ and generate_client_actions hash () =
| FileIn n
| FileOut n ->
(* guestfish doesn't support string escaping, so neither do we *)
pr " fprintf (trace_fp, \" \\\"%%s\\\"\", %s);\n" n
pr " fprintf (trace_buffer.fp, \" \\\"%%s\\\"\", %s);\n" n
| Key n ->
(* don't print keys *)
pr " fprintf (trace_fp, \" \\\"***\\\"\");\n"
pr " fprintf (trace_buffer.fp, \" \\\"***\\\"\");\n"
| OptString n -> (* string option *)
pr " if (%s) fprintf (trace_fp, \" \\\"%%s\\\"\", %s);\n" n n;
pr " else fprintf (trace_fp, \" null\");\n"
pr " if (%s)\n" n;
pr " fprintf (trace_buffer.fp, \" \\\"%%s\\\"\", %s);\n" n;
pr " else\n";
pr " fprintf (trace_buffer.fp, \" null\");\n"
| StringList n
| DeviceList n -> (* string list *)
pr " fputc (' ', trace_fp);\n";
pr " fputc ('\"', trace_fp);\n";
pr " fputc (' ', trace_buffer.fp);\n";
pr " fputc ('\"', trace_buffer.fp);\n";
pr " for (i = 0; %s[i]; ++i) {\n" n;
pr " if (i > 0) fputc (' ', trace_fp);\n";
pr " fputs (%s[i], trace_fp);\n" n;
pr " if (i > 0) fputc (' ', trace_buffer.fp);\n";
pr " fputs (%s[i], trace_buffer.fp);\n" n;
pr " }\n";
pr " fputc ('\"', trace_fp);\n";
pr " fputc ('\"', trace_buffer.fp);\n";
| Bool n -> (* boolean *)
pr " fputs (%s ? \" true\" : \" false\", trace_fp);\n" n
pr " fputs (%s ? \" true\" : \" false\", trace_buffer.fp);\n" n
| Int n -> (* int *)
pr " fprintf (trace_fp, \" %%d\", %s);\n" n
pr " fprintf (trace_buffer.fp, \" %%d\", %s);\n" n
| Int64 n ->
pr " fprintf (trace_fp, \" %%\" PRIi64, %s);\n" n
pr " fprintf (trace_buffer.fp, \" %%\" PRIi64, %s);\n" n
| BufferIn n -> (* RHBZ#646822 *)
pr " fputc (' ', trace_fp);\n";
pr " guestfs___print_BufferIn (trace_fp, %s, %s_size);\n" n n
pr " fputc (' ', trace_buffer.fp);\n";
pr " guestfs___print_BufferIn (trace_buffer.fp, %s, %s_size);\n" n n
| Pointer (t, n) ->
pr " fprintf (trace_fp, \" (%s)%%p\", %s);\n" t n
pr " fprintf (trace_buffer.fp, \" (%s)%%p\", %s);\n" t n
) args;
(* Optional arguments. *)
@@ -1119,25 +1121,25 @@ and generate_client_actions hash () =
(String.uppercase c_name) (String.uppercase n);
(match argt with
| OString n ->
pr " fprintf (trace_fp, \" \\\"%%s:%%s\\\"\", \"%s\", optargs->%s);\n" n n
pr " fprintf (trace_buffer.fp, \" \\\"%%s:%%s\\\"\", \"%s\", optargs->%s);\n" n n
| OStringList n ->
pr " fprintf (trace_fp, \" \\\"%%s:\", \"%s\");\n" n;
pr " fprintf (trace_buffer.fp, \" \\\"%%s:\", \"%s\");\n" n;
pr " for (i = 0; optargs->%s[i] != NULL; ++i) {\n" n;
pr " if (i > 0) fputc (' ', trace_fp);\n";
pr " fputs (optargs->%s[i], trace_fp);\n" n;
pr " if (i > 0) fputc (' ', trace_buffer.fp);\n";
pr " fputs (optargs->%s[i], trace_buffer.fp);\n" n;
pr " }\n";
pr " fputc ('\\\"', trace_fp);\n"
pr " fputc ('\\\"', trace_buffer.fp);\n"
| OBool n ->
pr " fprintf (trace_fp, \" \\\"%%s:%%s\\\"\", \"%s\", optargs->%s ? \"true\" : \"false\");\n" n n
pr " fprintf (trace_buffer.fp, \" \\\"%%s:%%s\\\"\", \"%s\", optargs->%s ? \"true\" : \"false\");\n" n n
| OInt n ->
pr " fprintf (trace_fp, \" \\\"%%s:%%d\\\"\", \"%s\", optargs->%s);\n" n n
pr " fprintf (trace_buffer.fp, \" \\\"%%s:%%d\\\"\", \"%s\", optargs->%s);\n" n n
| OInt64 n ->
pr " fprintf (trace_fp, \" \\\"%%s:%%\" PRIi64 \"\\\"\", \"%s\", optargs->%s);\n" n n
pr " fprintf (trace_buffer.fp, \" \\\"%%s:%%\" PRIi64 \"\\\"\", \"%s\", optargs->%s);\n" n n
);
pr " }\n"
) optargs;
pr " guestfs___trace_send_line (g);\n";
pr " guestfs___trace_send_line (g, &trace_buffer);\n";
pr " }\n";
pr "\n";
in
@@ -1156,43 +1158,43 @@ and generate_client_actions hash () =
pr "\n"
);
pr "%s trace_fp = guestfs___trace_open (g);\n" indent;
pr "%s guestfs___trace_open (&trace_buffer);\n" indent;
pr "%s fprintf (trace_fp, \"%%s = \", \"%s\");\n" indent name;
pr "%s fprintf (trace_buffer.fp, \"%%s = \", \"%s\");\n" indent name;
(match ret with
| RErr | RInt _ | RBool _ ->
pr "%s fprintf (trace_fp, \"%%d\", %s);\n" indent rv
pr "%s fprintf (trace_buffer.fp, \"%%d\", %s);\n" indent rv
| RInt64 _ ->
pr "%s fprintf (trace_fp, \"%%\" PRIi64, %s);\n" indent rv
pr "%s fprintf (trace_buffer.fp, \"%%\" PRIi64, %s);\n" indent rv
| RConstString _ | RString _ ->
pr "%s fprintf (trace_fp, \"\\\"%%s\\\"\", %s);\n" indent rv
pr "%s fprintf (trace_buffer.fp, \"\\\"%%s\\\"\", %s);\n" indent rv
| RConstOptString _ ->
pr "%s fprintf (trace_fp, \"\\\"%%s\\\"\", %s != NULL ? %s : \"NULL\");\n"
pr "%s fprintf (trace_buffer.fp, \"\\\"%%s\\\"\", %s != NULL ? %s : \"NULL\");\n"
indent rv rv
| RBufferOut _ ->
pr "%s guestfs___print_BufferOut (trace_fp, %s, *size_r);\n" indent rv
pr "%s guestfs___print_BufferOut (trace_buffer.fp, %s, *size_r);\n" indent rv
| RStringList _ | RHashtable _ ->
pr "%s fputs (\"[\", trace_fp);\n" indent;
pr "%s fputs (\"[\", trace_buffer.fp);\n" indent;
pr "%s for (i = 0; %s[i]; ++i) {\n" indent rv;
pr "%s if (i > 0) fputs (\", \", trace_fp);\n" indent;
pr "%s fputs (\"\\\"\", trace_fp);\n" indent;
pr "%s fputs (%s[i], trace_fp);\n" indent rv;
pr "%s fputs (\"\\\"\", trace_fp);\n" indent;
pr "%s if (i > 0) fputs (\", \", trace_buffer.fp);\n" indent;
pr "%s fputs (\"\\\"\", trace_buffer.fp);\n" indent;
pr "%s fputs (%s[i], trace_buffer.fp);\n" indent rv;
pr "%s fputs (\"\\\"\", trace_buffer.fp);\n" indent;
pr "%s }\n" indent;
pr "%s fputs (\"]\", trace_fp);\n" indent;
pr "%s fputs (\"]\", trace_buffer.fp);\n" indent;
| RStruct (_, typ) ->
(* XXX There is code generated for guestfish for printing
* these structures. We need to make it generally available
* for all callers
*)
pr "%s fprintf (trace_fp, \"<struct guestfs_%s *>\");\n"
pr "%s fprintf (trace_buffer.fp, \"<struct guestfs_%s *>\");\n"
indent typ (* XXX *)
| RStructList (_, typ) ->
pr "%s fprintf (trace_fp, \"<struct guestfs_%s_list *>\");\n"
pr "%s fprintf (trace_buffer.fp, \"<struct guestfs_%s_list *>\");\n"
indent typ (* XXX *)
);
pr "%s guestfs___trace_send_line (g);\n" indent;
pr "%s guestfs___trace_send_line (g, &trace_buffer);\n" indent;
pr "%s}\n" indent;
pr "\n";
in
@@ -1236,7 +1238,7 @@ and generate_client_actions hash () =
handle_null_optargs optargs c_name;
pr " int trace_flag = g->trace;\n";
pr " FILE *trace_fp;\n";
pr " struct trace_buffer trace_buffer;\n";
(match ret with
| RErr | RInt _ | RBool _ ->
pr " int r;\n"
@@ -1339,7 +1341,7 @@ and generate_client_actions hash () =
pr " int serial;\n";
pr " int r;\n";
pr " int trace_flag = g->trace;\n";
pr " FILE *trace_fp;\n";
pr " struct trace_buffer trace_buffer;\n";
(match ret with
| RErr | RInt _ | RBool _ -> pr " int ret_v;\n"
| RInt64 _ -> pr " int64_t ret_v;\n"

View File

@@ -77,37 +77,31 @@ guestfs___check_appliance_up (guestfs_h *g, const char *caller)
}
/* Convenience wrapper for tracing. */
FILE *
guestfs___trace_open (guestfs_h *g)
void
guestfs___trace_open (struct trace_buffer *tb)
{
assert (g->trace_fp == NULL);
g->trace_buf = NULL;
g->trace_len = 0;
g->trace_fp = open_memstream (&g->trace_buf, &g->trace_len);
if (g->trace_fp)
return g->trace_fp;
else
return stderr;
tb->buf = NULL;
tb->len = 0;
tb->fp = open_memstream (&tb->buf, &tb->len);
if (tb->fp)
tb->opened = true;
else {
tb->opened = false;
/* Fall back to writing messages to stderr. */
free (tb->buf);
tb->buf = NULL;
tb->fp = stderr;
}
}
void
guestfs___trace_send_line (guestfs_h *g)
guestfs___trace_send_line (guestfs_h *g, struct trace_buffer *tb)
{
char *buf;
size_t len;
if (g->trace_fp) {
fclose (g->trace_fp);
g->trace_fp = NULL;
/* The callback might invoke other libguestfs calls, so keep
* a copy of the pointer to the buffer and length.
*/
buf = g->trace_buf;
len = g->trace_len;
g->trace_buf = NULL;
guestfs___call_callbacks_message (g, GUESTFS_EVENT_TRACE, buf, len);
free (buf);
if (tb->opened) {
fclose (tb->fp);
tb->fp = NULL;
guestfs___call_callbacks_message (g, GUESTFS_EVENT_TRACE, tb->buf, tb->len);
free (tb->buf);
tb->buf = NULL;
}
}

View File

@@ -246,11 +246,6 @@ struct guestfs_h
struct hash_table *pda;
struct pda_entry *pda_next;
/* Used by src/actions.c:trace_* functions. */
FILE *trace_fp;
char *trace_buf;
size_t trace_len;
/* User cancelled transfer. Not signal-atomic, but it doesn't
* matter for this case because we only care if it is != 0.
*/
@@ -461,10 +456,17 @@ extern void guestfs___print_BufferOut (FILE *out, const char *buf, size_t buf_si
while (0)
/* actions-support.c */
struct trace_buffer {
FILE *fp;
char *buf;
size_t len;
bool opened;
};
extern int guestfs___check_reply_header (guestfs_h *g, const struct guestfs_message_header *hdr, unsigned int proc_nr, unsigned int serial);
extern int guestfs___check_appliance_up (guestfs_h *g, const char *caller);
extern FILE *guestfs___trace_open (guestfs_h *g);
extern void guestfs___trace_send_line (guestfs_h *g);
extern void guestfs___trace_open (struct trace_buffer *tb);
extern void guestfs___trace_send_line (guestfs_h *g, struct trace_buffer *tb);
/* match.c */
extern int guestfs___match (guestfs_h *g, const char *str, const pcre *re);