But maybe in lieu of my comments you can ack this patch and I'll
submit another one commenting this code and the pre-existing code that
does the same stuff? Would make things a bit easier.. ;-)
/Nils
On Nov 9, 2010, at 6:45 PM, Nils Carlson wrote:
Some commentary on the comments below... :-)
On Nov 9, 2010, at 5:28 PM, David Goulet wrote:
Comments below.
On 10-11-04 12:54 PM, Nils Carlson wrote:
Signed-off-by: Nils Carlson<[email protected]>
---
libustcomm/ustcomm.c | 32 ++++++++++++++++++++++++++++++++
libustcomm/ustcomm.h | 11 +++++++++++
2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
index fe8fea2..7d0fe00 100644
--- a/libustcomm/ustcomm.c
+++ b/libustcomm/ustcomm.c
@@ -621,6 +621,38 @@ char * ustcomm_restore_ptr(char *ptr, char
*data_field, int data_field_size)
return data_field + (long)ptr;
}
+int ustcomm_pack_trace_info(struct ustcomm_header *header,
+ struct ustcomm_trace_info *trace_inf,
+ const char *trace)
+{
+ int offset = 0;
+
+ trace_inf->trace = ustcomm_print_data(trace_inf->data,
+ sizeof(trace_inf->data),
+ &offset,
+ trace);
In order to understand this "data flow", I put some printf here to
output me the "trace_inf->trace" (that in my understanding, should
be the trace name with some other stuff...?) but each time it's NULL
It appears that you are passing "trace" to ustcomm_print_data and
it using it as a "format" and results in having :
trace_inf->trace = NULL
trace_inf->data = "auto"
I'm wondering if this is correct ? (Maybe a little comment on
print_data function to tell what kind of output it's generating?)
Can add a comment. What it does is that it packs the data and
assigns relative pointers, so the NULL there is the relative
position of trace_inf->trace within the data field. The unpack
function restores these pointers by adding the address of the data
field to each of them.
+
+ if (trace_inf->trace == USTCOMM_POISON_PTR) {
+ return -ENOMEM;
+ }
+
+ header->size = COMPUTE_MSG_SIZE(trace_inf, offset);
+
+ return 0;
+}
+
+
+int ustcomm_unpack_trace_info(struct ustcomm_trace_info *trace_inf)
+{
+ trace_inf->trace = ustcomm_restore_ptr(trace_inf->trace,
+ trace_inf->data,
+ sizeof(trace_inf->data));
+ if (!trace_inf->trace) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
int ustcomm_pack_channel_info(struct ustcomm_header *header,
struct ustcomm_channel_info *ch_inf,
diff --git a/libustcomm/ustcomm.h b/libustcomm/ustcomm.h
index f62250c..d548bc1 100644
--- a/libustcomm/ustcomm.h
+++ b/libustcomm/ustcomm.h
@@ -78,6 +78,11 @@ enum tracectl_commands {
STOP_TRACE,
};
+struct ustcomm_trace_info {
+ char *trace;
Would not be better to call the above variable something like
"trace_name" because it's becoming quite confusing in the code with
"trace" all over the place.
If so, maybe change the name all over the code from const char
*trace to const char *trace_name
Well, we use the term *marker and *channel to denote their names, so
it makes sense to do the same with *trace inside ustcomm.
The good news is then that ustcomm is internally consistent.
Will add a comment explaining how our home-made RPC works.
/Nils
Some other comment in the next patch but it follow these a bit.
Thanks
David
+ char data[USTCOMM_DATA_SIZE];
+};
+
struct ustcomm_channel_info {
char *channel;
unsigned int subbuf_size;
@@ -172,6 +177,12 @@ extern char * ustcomm_restore_ptr(char *ptr,
char *data_field,
(size_t) (long)(struct_ptr)->data - (long)(struct_ptr) + (offset)
/* Packing and unpacking functions, making life easier */
+extern int ustcomm_pack_trace_info(struct ustcomm_header *header,
+ struct ustcomm_trace_info *trace_inf,
+ const char *trace);
+
+extern int ustcomm_unpack_trace_info(struct ustcomm_trace_info
*trace_inf);
+
extern int ustcomm_pack_channel_info(struct ustcomm_header *header,
struct ustcomm_channel_info *ch_inf,
const char *channel);
--
David Goulet
LTTng project, DORSAL Lab.
PGP/GPG : 1024D/16BD8563
BE3C 672B 9331 9796 291A 14C6 4AF7 C14B 16BD 8563
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
