Michael Jeanson [Wed, 3 Jul 2019 22:09:56 +0000 (18:09 -0400)]
tests: replace xargs workaround with bash array expansion
Bash has a nice feature called 'Quoted array expansion' which we can
use to pass parameters between functions without convoluted quote
escaping. The syntax might not be extremmely clear at first though,
for example:
Which can be passed as a series of parameters to a function and then
converted back to a array for further use with :
local params=("$@")
This uses the special '$@' variable which is an array of all the
parameters to a function, 'shift' can be used to skip some parameters at
the beginning of the array.
An array can also be expanded as a single string with the members
separated by a space, with this syntax:
"${myarray[*]}"
Using this, I reworked the bt_diff functions to get rid of the xargs
call, I had to move the expected file parameter to the beginning so it
could be shifted and the rest of the parameters passed directly to CLI
executable.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Change-Id: Ie107c5259c9561fe158f923a53eeb0eaf7ee8b00
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1627 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Michael Jeanson [Thu, 27 Jun 2019 21:41:53 +0000 (17:41 -0400)]
Replace libuuid with internal implementation
We use a very small subset of libuuid features to transform UUIDs
between their string and binary representation. Plus we have a lot of
compat code for different platforms with some unspecified default
behavior regarding the use of upper/lower case.
Drop the dependency on libuuid and replace it with a minimal internal
implementation that respects RFC4122.
Change-Id: Ic170ce26ade23d177195cad117bd0fab590b328e Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1572 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 20 Jun 2019 21:14:46 +0000 (17:14 -0400)]
lib: remove stream activity messages
The stream activity messages were introduced to support eventual use
cases where the tracer could indicate when it stopped tracing and
started again. To this day, no tracer which produces CTF provides this
information, so these messages are not currently useful. Stream
activity beginning messages are only ever sent just after stream
beginning, and stream activity end messages are only ever sent just
before stream end.
Given that we will introduce a versioning system for the inter-component
communication protocol, which will allow adding new features, we can
remove the stream activity messages for now and add them at a latter
time, if need be. This will reduce a little bit the amount of
boilerplate needed to implement a simple source.
So this patch removes the stream activity messages and handles all the
fallout.
One thing is that the stream activity messages optionally carried a
default clock snapshot, which is used by the trimmer component. To
compensate for this loss of feature, the stream beginning/end messages
now have an optional default clock snapshot.
Impacts around the codebase include (on top of simply removing handling
of stream activity messages):
- sink.text.details: Update to include the default clock snapshot for
stream beginning/end.
- flt.utils.trimmer: read clock snapshots from stream beginning/end
messages (if not unknown). These messages can now trigger the end of
the trimming, if their default clock snapshot is greater than the end
time of the trimmer. Set the clock snapshot of the generated stream
end message if the stream is ended by the trimmer's end bound (see
note below for more details). If the stream is ended because of a
stream end message that is within the trimmer's bounds, the stream
end message is not modified.
- iterator: handle stream message clock snapshots when validating clock
monotonicity and doing an auto seek.
Note about clock snapshot handling
----------------------------------
This patch fixes a bug in trimmer's handle_message_with_stream function,
where it currently always returns BT_SELF_MESSAGE_ITERATOR_STATUS_OK,
even when something fails. Changing it to return the "status" variable
(as I suppose the original intention was) uncovers a latent bug with
the clock snapshots of the "stream end" messages we
generate (or for stream activity end messages, before this patch).
Imagine a clock with a frequency of 1 and offset of 1000s. This means
that the raw clock value 0 corresponds to the real time 1000s, 50 is
1050, and so on. This clock is unable to represent real times prior to
1000s. The sequence of messages generated by the source is:
1. Stream beginning, no clock snapshot
2. Packet beginning at 1050s (raw value 50)
Here's what happens in the trimmer's mind if we pass --end=200:
- Receive message 1, record that a stream is open
- Receive message 2, realize its clock snapshot is greater than the end
bound. Drop it and end the streams.
- Generate a stream end message, use own end bound (200s) as the clock
snapshot
- Try to convert 200s to a raw value of the clock described above: it
fails because 200s can't be represented by that clock.
This is fixed by recording whether we have seen a message with a valid
clock snapshot associated with the stream (which can therefore be
represented by the stream's class' clock class). If we have, it means
that the clock snapshot we'll choose for the stream end message we'll
generate will be safe to convert to a clock value of that clock class.
If we haven't, we are not sure.
A similar problem happens in the iterator auto-seek code when using
'--begin=200' and the same setup as above. We'll try to generate a
stream beginning message with clock snapshot 200s, which is invalid.
Note that this approach could be problematic in some corner cases,
here's a known one:
Once trimmer supports packet beginning/end messages without clock
snapshots (remember that whether these messages have a clock
snapshot or not is dictated by the stream class, it's not a choice per
message), this could happen:
- stream class defines that packet beginning messages don't have clock
snapshots, but packet end messages do.
- get stream beginning message without clock snapshot
- get packet beginning message without clock snapshot
- get event message with clock snapshot, greater than the end bound of
200s
- since the stream class defines that packet end messages must have a
clock snapshot, we can't get away by not putting one as is done by
this patch.
A more complex solution along these lines would probably be needed:
- When trimmer receives the stream and packet beginning messages without
clock snapshot, don't send them downstream but take a note of the
state of the stream
- When we do get a first message with a clock snapshot, then send the
required stream and packet beginning messages.
- If the trimming ends without having sent any content in this
stream/packet, just pretend it never existed.
Change-Id: I9a30a4c33b3f94497254ef93b24fed3b463e13fa Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1527 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 3 Jul 2019 19:23:15 +0000 (15:23 -0400)]
tests: don't swallow stderr when running babeltrace CLI
I have found it very difficult to investigate failing tests if we don't
show stderr, so I suggest we just let it get printed. Some tests are
expected to print to stderr in their normal course of operation (because
they test babeltrace error cases), so it means there will be some
additional output in a normal test run. But I think it's a fair
trade-off.
Change-Id: Ia9bc5a706dd756ce0c9d1be4fec7510b12dface7 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1609
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Philippe Proulx [Fri, 5 Jul 2019 19:11:11 +0000 (15:11 -0400)]
lib: move trace class's name, UUID, and environment props to trace API
This patch moves the name, UUID, and environment properties of the trace
class API to the trace API. The rationale behind this is that those
properties fundamentally belong to the instance of a trace class, not to
the class itself, as many traces with different names, UUIDs, and
environments can be created from a single trace class.
With this patch, a trace class becomes a simple container of stream
classes. Its name property is removed because it's not needed currently.
Specific, non-trivial changes:
`ctf` plugin:
ctf_trace_class_translate() does not translate the UUID and
environment anymore. Those properties are still part of a CTF IR
trace class however. Instead, the new
ctf_trace_class_configure_ir_trace() function can configure a trace
IR trace object from the properties of a CTF IR trace class object.
Both `src.ctf.fs` and `src.ctf.lttng-live` use
ctf_trace_class_configure_ir_trace().
`sink.ctf.fs`:
The private CTF IR trace class object is renamed to
`fs_sink_ctf_trace` as it represents a single trace anyway.
Philippe Proulx [Thu, 4 Jul 2019 06:00:18 +0000 (02:00 -0400)]
cli: print current thread's error causes, if any, before exiting
This patch makes the CLI print the error causes of the current thread's
error object, if any, before it finally exits.
The causes are printed from the most recent to the least recent. In
other words, the root cause, or deepest cause, is printed at the end of
the list. This is similar to what Python does when it prints a
traceback.
The CLI prints error causes with colors if supported to highlight the
cause's module name, specific properties, file name, and line number.
The CLI prints the messages indented with two spaces and folded on the
terminal's current width if available, otherwise on 80 columns.
You can test the output with
babeltrace2 $(uuidgen)
This prints something like:
ERROR: [Babeltrace CLI] (babeltrace2.c:2531)
Cannot create components.
CAUSED BY [Babeltrace CLI] (babeltrace2.c:2355)
Cannot create component: plugin-name="ctf", comp-cls-name="fs",
comp-cls-type=0, comp-name="source-ctf-fs"
CAUSED BY [Babeltrace library] (graph.c:1336)
Component initialization method failed: status=ERROR,
comp-addr=0x5590750b8fa0, comp-name="source-ctf-fs",
comp-log-level=BT_LOGGING_LEVEL_WARN,
comp-class-type=BT_COMPONENT_CLASS_TYPE_SOURCE, comp-class-name="fs",
comp-class-partial-descr="Read CTF traces from the file sy",
comp-class-is-frozen=0, comp-class-so-handle-addr=0x5590750b2aa0,
comp-class-so-handle-path="babeltrace2/plugins/babeltrace-plugin-ctf.so",
comp-input-port-count=0, comp-output-port-count=0
CAUSED BY [source-ctf-fs: 'source.ctf.fs'] (fs.c:1320)
No CTF traces recursively found in
`6419ec89-991d-4cf7-ab7f-b143a9901562`.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ib8101e097acf98db305775ac5b90f4eb006ce4ff
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1622 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 21 Jun 2019 18:17:29 +0000 (14:17 -0400)]
lib: use BT_LIB_LOG*_APPEND_CAUSE() where appropriate
Use BT_LIB_LOGE_APPEND_CAUSE() or BT_LIB_LOGW_APPEND_CAUSE() where
appropriate, that is, where a public library function eventually fails
(returns an error status code or `NULL` for a creation function).
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I86f8588accfdd7e0c8b69e3977d214d30ccbd67d
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1618 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
The new BT_LIB_LOGE_APPEND_CAUSE() and BT_LIB_LOGF_APPEND_CAUSE()
functions are like BT_LIB_LOGE() and BT_LIB_LOGF(), but they also call
bt_current_thread_error_append_cause_from_unknown() with the formatted
message to append a cause to the current thread's error object.
I plan to use them in lieu of BT_LIB_LOGE() and BT_LIB_LOGF() where
those are already used within the library to make support error
reporting from the library with minimal changes.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I6d7e3a5feac977f576c29c2451b5f0da8d42ae2b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1617 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Wed, 3 Jul 2019 07:17:37 +0000 (03:17 -0400)]
cli: introduce BT_CLI_LOG*_APPEND_CAUSE() and use it where appropriate
BT_CLI_LOGE_APPEND_CAUSE() and BT_CLI_LOGW_APPEND_CAUSE() are internal
CLI macros which log and append an error cause with the same message,
using "Babeltrace CLI" as the cause's module name.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I2202ce81582860247232cd3aeea82aa3561e1227
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1616 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Philippe Proulx [Wed, 3 Jul 2019 20:31:56 +0000 (16:31 -0400)]
plugins: call bt_current_thread_clear_error() when not propagating error
Most of the locations where bt_current_thread_clear_error() is called in
this patch need to be replaced with proper error handling, except in
finalization methods which cannot fail. This work will be done by a
subsequent patch.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I10fb2eb98f46e685e6abf563346a2750b46b01d9
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1613 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Sat, 15 Jun 2019 20:48:07 +0000 (16:48 -0400)]
lib: add thread-safe error reporting API
This patch adds an API for rich error reporting from an internal module
(library, Python plugin provider, etc.), from a component, from a
component class (query operation), from a message iterator, or from a
graph listener function.
The goal of this is, as a library user, to have access to an organized
list of causes leading to a specific error when a library function
fails. Those causes have module name and message properties, and can
occur within user code. This is similar to a stack trace carried by an
exception object in other languages.
The objective is that a program (the CLI, for example) can print a
report like this (module name between square brackets):
ERROR: Graph failed to run completely:
[ctf1 (stream0): src.ctf.fs] Cannot decode packet header for packet
#546 in file `/path/to/stream/file`:
invalid packet size (17 bits).
CAUSED:
[ctf1 (stream0): src.ctf.fs] Cannot get next message.
CAUSED:
[Babeltrace library] Message iterator's "next" method failed; port
`port-name` of component `ctf1` (`src.ctf.fs`).
CAUSED:
[mux (out): flt.utils.muxer] Cannot get next upstream message
iterator's message for input port `in3`.
CAUSED:
[Babeltrace library] Message iterator's "next" method failed; port
`out` of component `mux` (`flt.utils.muxer`).
CAUSED:
[txt: sink.text.pretty] Cannot get next upstream message iterator's
message.
CAUSED:
[Babeltrace library] Sink component's "consume" method failed.
CAUSED:
[Babeltrace library] Graph stopped running.
The new concepts are:
Error cause actor:
The actor which is responsible for the cause of an error. The
available actors are:
Unknown:
No specific actor (the library will use this, for example, as
well as graph listeners).
Component:
The user code of a component caused the error.
Component class:
The user code of a component class (during a query operation)
caused the error.
Message iterator:
The user code of a message iterator caused the error.
Error cause:
The cause of an error, that is, the context in which the error
occured. An error cause has an actor type, a module name, a message,
and, depending on its actor, other properties.
For example, a single error cause could contain a whole Python stack
trace as its message.
The API function names start with `bt_error_cause`.
Error:
A list of error causes.
The API function names start with `bt_error`.
Current thread:
Everything related to the current thread. The only available
property is its error object.
The API function names start with `bt_current_thread`.
The library header `error-const.h` contains what you need to borrow the
causes of an error object: bt_error_get_cause_count() returns the number
of contained error causes, and with bt_error_borrow_cause_by_index() you
can borrow a specific error cause.
When you call a library function and it returns an error status, you
MUST either:
* Handle the error, and then continue. In that case, you need to clear
the current thread's error object somehow.
If you don't care about the causes of the error, you can call
bt_current_thread_clear_error().
If you need to check the causes of the error, call
bt_current_thread_take_error() which transfers the ownership of the
current thread's error to you, clearing the thread's error at the same
time. This is similar, for example, to how PyErr_Fetch() retrieves the
error indicator and then clears it.
Once you're done with the error object you now own, you can either:
* Discard it with bt_error_release().
* Move it back to the current thread as its error object with
bt_current_thread_move_error().
* Propagate the error, that is, return an error status yourself.
Optionally, you can append an error cause (from your context) to the
current thread's error object with one of the
bt_current_thread_error_append_cause_from_*() functions or
BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_*() macros (more details
below).
In that case, the error will be handled as explained above by one of
your callers.
The error cause API, depending on the error cause actor, offers the
component name, component class name and type, plugin name (if
available), and component output port name.
I'm not comfortable keeping a weak reference on the component, component
class, plugin, or message iterator object within the error cause object
itself because the error could possibly outlive the component object.
I'm also not comfortable keeping a strong reference because I'm pretty
sure someone will forget to call bt_current_thread_clear_error() or
bt_error_release() eventually and in that case it could cause important
leaks. Leaking a few strings is not the end of the world, but if you
leak the whole graph, then some finalization methods won't be called and
you could have some serious incompleteness.
You cannot append error causes to any error object because this would
require taking the current thread's error object and then moving it back
each time you need to append an error cause.
Instead, you can append an error cause to the current thread's error
specifically with one of:
The macro versions call the non-macro versions with `__FILE__` and
`__LINE__` for the `file_name` and `line_no` parameters.
The functions above work like printf() from their `msg_fmt` argument to
set the error cause's message.
For bt_current_thread_error_append_cause_from_unknown(), the module name
can be anything. It should clearly identify the actor (for example,
`Babeltrace library`, `Babeltrace CLI`, `Python bindings`). When the
error cause's actor is not unknown, then the module name is conveniently
set automatically:
Component:
`my-comp: src.ctf.fs`
Component class:
`src.ctf.fs` or `src.comp-cls` (no plugin)
Message iterator:
`my-comp (out2): src.ctf.fs`
As of this patch, the projet does not use the new API. This work is
reserved for subsequent patches.
Philippe Proulx [Wed, 19 Jun 2019 21:08:09 +0000 (17:08 -0400)]
lib: keep plugin name, if any, in component class structure
This is to make the plugin name available when having a component class
object in an upcoming error reporting API.
We don't keep the `bt_plugin` object itself because it's a component
class's owner, and a component class can outlive its containing plugin:
only the shared object data is kept alive in that case.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I9cfbac32d88a6e7edac238b5a63b232695613b8c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1523 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
src.ctf.fs: remove leftover use of pointer arithmetics
This change should have been include in the commit:
commit de38c26a14b60cb3b6d31cc124c187e2c1816bf5
Author: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Fri Jul 5 11:19:32 2019 -0400
Fix: src.ctf.fs: pointer arithmetics on non-adjacent memory
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: If44dc7f5330f070c865b241d507082e253a92a4b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1641 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
bt2: namespace private attributes of inheritable classes
Add `_bt` prefix to all private attributes and methods of classes of
which the user can inherit. This is done to prevent attribute name
clashes between the base class and the user class. We will document that
the user must not have any attribute with the `_bt_` prefix.
Here are the attribute and method names of inheritable classes with this
commit:
_UserSinkComponent:
self._add_input_port
self._bt_as_component_class_ptr
self._bt_as_component_ptr
self._bt_as_not_self_specific_component_ptr
self._bt_as_self_component_ptr
self._bt_borrow_component_class_ptr
self._bt_cc_ptr
self._bt_comp_cls_type
self._bt_graph_is_configured_from_native
self._bt_port_connected_from_native
self._bt_ptr
self._consume
self._create_clock_class
self._create_trace_class
self._finalize
self._input_ports
self._port_connected
self.addr
self.cls
self.logging_level
self.name
Fix: src.ctf.fs: segfault following `bt_msg_iter_seek()`
Issue
=====
Seeking a `bt_msg_iter` right after its creation leaves its `mmap_addr`
field uninitialized which may lead to a segmentation fault when
`medop_request_byte()` is called.
It triggers a segfault in `medop_request_byte()` because
`ds_file->mmap_len` field is 0 and `ds_file->request_offset` field is
larger than 0. This makes the call to `remaining_mmap_bytes()` return
non-zero.
It makes it so that the call to `ds_file_mmap_next()` is skipped and
`ds_file->mmap_addr` is dereferenced without being initialized.
The real underlying issue is that `medop_seek()` returns success even
when no file is mapped.
Solution
========
When seeking, map the right file if `mmap_addr` is still uninitialized.
Drawback
========
None.
Notes
=====
I also added comments and `BT_ASSERT()` that helped me track down this
issue and that I believe could be useful for other developers.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I041dc89159953a6ffce016b8c094969fb9c3f862
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1532 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
Valgrind report:
==21704== 256 bytes in 4 blocks are definitely lost in loss record 29 of 38
==21704== at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21704== by 0x555EA1F: g_realloc (gmem.c:164)
==21704== by 0x557AF56: g_string_maybe_expand (gstring.c:102)
==21704== by 0x557B2E4: g_string_insert_len (gstring.c:476)
==21704== by 0x557B44C: g_string_assign (gstring.c:349)
==21704== by 0x6B2D8C5: ctf_fs_make_port_name (fs.c:427)
==21704== by 0x6B31FCD: populate_stream_info (query.c:342)
==21704== by 0x6B321E9: populate_trace_info (query.c:417)
==21704== by 0x6B32592: trace_info_query (query.c:525)
==21704== by 0x6B3104C: ctf_fs_query (fs.c:2326)
==21704== by 0x4E7ED6A: bt_query_executor_query (query-executor.c:128)
==21704== by 0x10DCDA: query (babeltrace2.c:201)
Reported-by: Valgrind Memcheck Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I4e8ac593327d170005fd984ef7744656e7feb47d
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1419 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Fri, 5 Jul 2019 14:58:45 +0000 (10:58 -0400)]
bt2: remove probing of BABELTRACE_PYTHON_BT2_NO_TRACEBACK env var
This was used previously when handling exceptions thrown by user code,
to decide whether we printed exceptions or not. Nowadays, we let
exceptions propagate to the native code, which logs it. I believe we
can remove this.
Change-Id: I14adae265c83a169d176dda9d83d4d05fcd26edc Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1636 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 4 Jul 2019 20:31:00 +0000 (16:31 -0400)]
trimmer: use g_match_info_free instead of g_match_info_unref
The build fails with glib 2.22:
trimmer.c: In function ‘compile_and_match’:
trimmer.c:201:3: error: implicit declaration of function ‘g_match_info_unref’; did you mean ‘g_match_info_free’? [-Werror=implicit-function-declaration]
g_match_info_unref(*match_info);
^~~~~~~~~~~~~~~~~~
g_match_info_free
The g_match_info_unref function is only available from glib 2.30. Use
g_match_info_free instead, which does the same for our use case.
Change-Id: I9357b9dd18501749e19481e9ef08ec9b01379ee6 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1633 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
This commit adds a tiny testing section containing an introduction and a
few useful commands to run the `bt2` tests.
Philippe will without a doubt rework this section upon the release of
Babeltrace 2.0. So this commit simply adds commands useful to developers
in the meantime.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: If4be315ddcc09efe9224e42de62c5b01c73ead8f
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1602 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
tests: bt2: add `--test-case` argument to testrunner.py
The `-t/--test-case` argument allows to specify the exact test case to
run.
Make the `--test-case` and the existing `--pattern` options mutually
exclusive and require that one of the two is provided.
It can be used like this:
./tests/utils/run_python_bt2 python3 ./tests/utils/python/testrunner.py \
-t test_event.EventTestCase.test_clock_value \
./tests/bindings/python/bt2
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Icc9204d05bac4cef9cc509f2b19a9e515b0a40e7
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1612 Reviewed-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
Fix: src.ctf.fs: pointer arithmetics on non-adjacent memory
Issue
=====
When indexing a trace using `*.idx` files, if streams span more than one
packet the user may witness the following warning message:
W PLUGIN/SRC.CTF.FS/DS build_index_from_idx_file@data-stream-file.c:400
[source-ctf-fs] Invalid, non-monotonic, packet offset encountered in
LTTng trace index file: previous offset=14757395258967641292, current offset=4096
This is caused by the fact the we're using pointer arithmetics to get
the pointer to the previous entry of an array. This ends returning
garbage because it's a pointer array and not a regular array storing the
objects as the code expects to.
This regression was most probably introduced by the following commit:
commit 7ed5243afe12c7c12fa5305fff99b93ad23bbdff
Author: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Wed May 15 14:59:10 2019 -0400
src.ctf.fs: merge all indexes to the fs_ds_group level
This problematic commit changes the `entries` fields of the `struct
ctf_fs_ds_index` from a `GArray *` to `GPtrArray * without changing the
pointer arithmetics.
This was not an error because indexing using `*.idx` files is optional
and indexing may fail back to direct packet indexing.
Solution
========
Use a local variable to store a pointer to the previous index_entry
rather than using pointer arithmetics.
Drawbacks
=========
None.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I3e5608d0359be2a447e79415517068c64f5a2817
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1637 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Philippe Proulx [Thu, 4 Jul 2019 07:04:20 +0000 (03:04 -0400)]
Fix: sink.text.pretty: support unknown event class name
An event class's name property is optional. Add support in
`sink.text.pretty` for a nonexistent event class name. The component
prints `<unknown>`, the same way it prints this for an enumeration field
value without a corresponding mapping.
libtool wrapper fail to resolve executable without ".exe" suffix on
mingw system.
tests/ctf-writer/test_ctf_writer:
babeltrace2:./.libs/lt-babeltrace2.c:237: FATAL: couldn't find ./../utils/../../src/cli/babeltrace2.
not ok 325 - Babeltrace could read the resulting trace
Change-Id: I4e916077925b34745e6b70d14d86e263427d081e Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1611 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com>
mman.c: In function 'mmap_lock':
mman.c:29:30: error: 'mapping' undeclared (first use in this function)
29 | #define BT_LOG_OUTPUT_LEVEL (mapping->log_level)
| ^~~~~~~
../../src/logging/log.h:165:31: note: in expansion of macro 'BT_LOG_OUTPUT_LEVEL'
165 | #define _BT_LOG_OUTPUT_LEVEL BT_LOG_OUTPUT_LEVEL
| ^~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:671:38: note: in expansion of macro '_BT_LOG_OUTPUT_LEVEL'
671 | (BT_LOG_ENABLED((lvl)) && (lvl) >= _BT_LOG_OUTPUT_LEVEL)
| ^~~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:835:9: note: in expansion of macro 'BT_LOG_ON'
835 | if (BT_LOG_ON(lvl)) \
| ^~~~~~~~~
../../src/logging/log.h:974:4: note: in expansion of macro 'BT_LOG_WRITE'
974 | BT_LOG_WRITE(BT_LOG_FATAL, _BT_LOG_TAG, __VA_ARGS__)
| ^~~~~~~~~~~~
../../src/logging/log.h:995:24: note: in expansion of macro 'BT_LOGF'
995 | #define BT_LOGF_STR(s) BT_LOGF("%s", (s))
| ^~~~~~~
mman.c:122:3: note: in expansion of macro 'BT_LOGF_STR'
122 | BT_LOGF_STR("Failed to acquire mmap_mutex.");
| ^~~~~~~~~~~
mman.c:29:30: note: each undeclared identifier is reported only once for each function it appears in
29 | #define BT_LOG_OUTPUT_LEVEL (mapping->log_level)
| ^~~~~~~
../../src/logging/log.h:165:31: note: in expansion of macro 'BT_LOG_OUTPUT_LEVEL'
165 | #define _BT_LOG_OUTPUT_LEVEL BT_LOG_OUTPUT_LEVEL
| ^~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:671:38: note: in expansion of macro '_BT_LOG_OUTPUT_LEVEL'
671 | (BT_LOG_ENABLED((lvl)) && (lvl) >= _BT_LOG_OUTPUT_LEVEL)
| ^~~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:835:9: note: in expansion of macro 'BT_LOG_ON'
835 | if (BT_LOG_ON(lvl)) \
| ^~~~~~~~~
../../src/logging/log.h:974:4: note: in expansion of macro 'BT_LOG_WRITE'
974 | BT_LOG_WRITE(BT_LOG_FATAL, _BT_LOG_TAG, __VA_ARGS__)
| ^~~~~~~~~~~~
../../src/logging/log.h:995:24: note: in expansion of macro 'BT_LOGF'
995 | #define BT_LOGF_STR(s) BT_LOGF("%s", (s))
| ^~~~~~~
mman.c:122:3: note: in expansion of macro 'BT_LOGF_STR'
122 | BT_LOGF_STR("Failed to acquire mmap_mutex.");
| ^~~~~~~~~~~
mman.c: In function 'mmap_unlock':
mman.c:29:30: error: 'mapping' undeclared (first use in this function)
29 | #define BT_LOG_OUTPUT_LEVEL (mapping->log_level)
| ^~~~~~~
../../src/logging/log.h:165:31: note: in expansion of macro 'BT_LOG_OUTPUT_LEVEL'
165 | #define _BT_LOG_OUTPUT_LEVEL BT_LOG_OUTPUT_LEVEL
| ^~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:671:38: note: in expansion of macro '_BT_LOG_OUTPUT_LEVEL'
671 | (BT_LOG_ENABLED((lvl)) && (lvl) >= _BT_LOG_OUTPUT_LEVEL)
| ^~~~~~~~~~~~~~~~~~~~
../../src/logging/log.h:835:9: note: in expansion of macro 'BT_LOG_ON'
835 | if (BT_LOG_ON(lvl)) \
| ^~~~~~~~~
../../src/logging/log.h:974:4: note: in expansion of macro 'BT_LOG_WRITE'
974 | BT_LOG_WRITE(BT_LOG_FATAL, _BT_LOG_TAG, __VA_ARGS__)
| ^~~~~~~~~~~~
../../src/logging/log.h:995:24: note: in expansion of macro 'BT_LOGF'
995 | #define BT_LOGF_STR(s) BT_LOGF("%s", (s))
| ^~~~~~~
mman.c:131:3: note: in expansion of macro 'BT_LOGF_STR'
131 | BT_LOGF_STR("Failed to release mmap_mutex.");
| ^~~~~~~~~~~
mman.c: At top level:
mman.c:173:7: error: conflicting types for 'bt_mmap'
173 | void *bt_mmap(void *addr, size_t length, int prot, int flags, int fd,
| ^~~~~~~
In file included from mman.c:53:
../../src/compat/mman.h:48:7: note: previous declaration of 'bt_mmap' was here
48 | void *bt_mmap(void *addr, size_t length, int prot, int flags, int fd,
| ^~~~~~~
mman.c: In function 'bt_mmap':
mman.c:202:12: error: too many arguments to function 'mapping_create'
202 | mapping = mapping_create(log_level);
| ^~~~~~~~~~~~~~
mman.c:73:22: note: declared here
73 | struct mmap_mapping *mapping_create(void)
| ^~~~~~~~~~~~~~
make[2]: *** [Makefile:501: mman.lo] Error 1
Change-Id: I71b36504541382cadf09953d9e113adc6c3641b6 Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1610 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 3 Jul 2019 19:46:14 +0000 (15:46 -0400)]
tests: add python plugin provider path to lib search path
I am working on a test that uses a component class implemented in a
Python plugin. For it to work, babeltrace therefore needs to be able to
load the python plugin provider lib. It currently isn't found:
LIB/PLUGIN init_python_plugin_provider@plugin.c:84 Loading Python plugin provider module.
LIB/PLUGIN init_python_plugin_provider@plugin.c:94 Cannot open `libbabeltrace2-python-plugin-provider.so`: libbabeltrace2-python-plugin -provider.so: cannot open shared object file: No such file or directory: continuing without Python plugin support.
This patch makes run_python_bt2 add the right path to LD_LIBRARY_PATH
(PATH on Windows) so that babeltrace can load python plugins.
With this added we don't need the feature of run_python_bt2 and
run_python_bt2 that allowed to append an additional directory to
LD_LIBRARY_PATH, so it is removed.
Change-Id: Ie72f59c9ef4866c7f0eae0e6f49025d2fa1ba127 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1608
CI-Build: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
Simon Marchi [Thu, 6 Jun 2019 20:59:44 +0000 (16:59 -0400)]
Fix: trimmer: use regexes to parse dates and times
This patch fixes a few issues related to how the trimmer component
parses timestamps.
1. Less than 9 digits in the nanoseconds position is not interpreted
correctly.
Passing "2019-01-02 12:34:56.444", we expect the nanosecond portion
to mean 444 ms, or 444000000 ns. Currently, it is interpreted as 444
ns.
2. We wrongfully accept missing digits in the various fields
It is currently possible to pass a date as "2019-1-2" (instead of
"2019-01-02") or a time as "1:2:3" (instead of "01:02:03"). This
patch makes the parsing stricter, in order to only accept the right
number of digits when parsing dates (in the form "YYYY-MM-DD") and
times (in the form "HH:MM:SS"). The nanosecond portion, if present,
can contain anywhere from 1 to 9 digits.
Change-Id: Ie315cec0bc7387ecdaa999d6c5f82f70035412f3 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1389
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 3 Jul 2019 15:40:49 +0000 (11:40 -0400)]
tests: Use diff -u
We are all more used to "unified" diff output formats than other
formats, so I suggest we use that for differences between expected and
actual results.
Change-Id: I681fcd232459eedf9c084514ca9d54be9732adcc Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1601 Reviewed-by: Jonathan Rajotte Julien <jonathan.rajotte-julien@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
CI-Build: Michael Jeanson <mjeanson@efficios.com> Tested-by: jenkins <jenkins@lttng.org>
Philippe Proulx [Tue, 2 Jul 2019 21:16:27 +0000 (17:16 -0400)]
Allow only TRACE, DEBUG, and INFO as the build-time, minimal log level
As there's no noticeable performance difference between having the INFO
log level and the FATAL log level as the build's minimal log level,
force it to be TRACE, DEBUG, or INFO. This means that INFO, WARN, ERROR,
and FATAL logging is always enabled, whatever the build configuration.
This seems reasonable and could prove useful to support/debugging
operations.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ie4261fd16d344587ef4b37ea1715d7302ad25087
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1590
Philippe Proulx [Sun, 30 Jun 2019 06:26:38 +0000 (02:26 -0400)]
lib: strictly type function return status enumerations
Rigour strikes again.
This patch makes each function (or group of related functions) of the
library which returns a status enumeration return its own,
function-specific status enumeration.
For example, where bt_graph_run() used to return the generic
`bt_graph_status` type, it now returns `bt_graph_run_status`.
The benefits are:
* Each function defines its own set of available status codes. With
appropriate compiler warnings, you can catch comparisons between a
returned status and an incompatible enumerator, and assignments of
incompatible enumerators. For example, with clang 8:
file.c:13:13: warning: implicit conversion from enumeration type
'enum bt_component_class_init_method_status'
to different enumeration type
'enum bt_component_class_port_connected_method_status'
[-Wenum-conversion]
bt_component_class_port_connected_method_status status =
BT_COMPONENT_CLASS_INIT_METHOD_STATUS_ERROR;
This was not possible before because all the status-returning
functions of a given header could possibly return any value defined by
the header-wide status enumeration.
* A function's possible return values are self-documented: there's no
need to document that a given function can only return some subset of
the header-wide status enumerators.
* In future minor releases of Babeltrace 2, we can add new status codes
for a specific function.
* Some smart IDEs will list the possible enumerators when checking the
return value of a function or when returning a value from a function.
To uniformize the status enumerator integer values, the new
`<babeltrace2/func-status.h>` public header contains definitions for all
the possible status codes. They are definitions instead of enumerators
because they are not meant to be used directly by user code.
To make sure a user does not write:
#include <babeltrace2/func-status.h>
to use the `__BT_FUNC_STATUS_*` definitions directly (which would
inhibit the precious type checking that this patch is all about), the
compiler will show the error:
> Do NOT include <babeltrace2/func-status.h> in user code.
unless you define `__BT_FUNC_STATUS_ENABLE` before including the header.
Each public header which needs `__BT_FUNC_STATUS_*` definitions to
define its own status enumerations does:
/* For __BT_FUNC_STATUS_* */
#define __BT_FUNC_STATUS_ENABLE
#include <babeltrace2/func-status.h>
#undef __BT_FUNC_STATUS_ENABLE
at the beginning of the file, and:
#include <babeltrace2/undef-func-status.h>
at the end of the file. The `<babeltrace2/undef-func-status.h>` header
cancels all the existing `__BT_FUNC_STATUS_*` definitions so that they
are not available to user code.
Within the library, `"lib/func-status.h"` uses the definitions of
`<babeltrace2/func-status.h>` to define its own, similar definitions,
but without the `__` prefix for improved internal code readability.
Internal library code does not need to "undefine" those definitions.
As this system guarantees that any public status enumerator is an alias
of one of the `__BT_FUNC_STATUS_*` definitions, the library code never
uses the function-specific status enumerators directly: it always uses
one of the `BT_FUNC_STATUS_*` definitions found in
`"lib/func-status.h"`. This made it easier to adapt existing code. This
also allows to pass such status codes easily between internal and public
functions because they are `int` values until they finally reach public
enumeration types where they are implicitly casted.
When an internal (hidden, static) function returns a status code, it now
returns `int`, using one of the `BT_FUNC_STATUS_*` values, where this
could be used by at least two functions returning different public
status enumerations.
In internal headers, all the bt_*_status_string() are removed in favor
of bt_common_func_status_string() which returns only the suffix string
(e.g., `OK`, `ERROR`, `OVERFLOW`). This function is typically used when
logging.
The `*_NOMEM` enumerators are renamed to `*_MEMORY_ERROR` to make this
clear and remove this abbreviation which was the only one amongst other
public status enumerators.
Because the Python bindings are within the Babeltrace project, they use
the `__BT_FUNC_STATUS_*` definitions internally. Those values will not
change in the future anyway. All the _handle_*status() and
bt2.utils._handle_ret() functions are removed in favor of
bt2.utils._handle_func_status() which can handle all the possible status
codes and raise a corresponding exception, possibly with a given
message.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I3966ac0e691219102897920617711c518c34fbdf
In bt_packet_context_field_create(), it is not a precondition that the
stream class be frozen. It used to mean that the stream class was
previously added to a trace class, but this is always the case now that
you pass the trace class to bt_stream_class_create().
bt_packet_context_field_create() freezes the stream class on success.
This shows that you can call bt_packet_context_field_create() with a
fresh, hot stream class, and then it becomes frozen.
Philippe Proulx [Sat, 29 Jun 2019 16:30:09 +0000 (12:30 -0400)]
test_{field,value}.py: array test cases: add non-sequence equality tests
The new tests verify that an array field or an array value is not equal
to another iterable object which would provide the correct element
values, but which is not a `collections.abc.Sequence`. This is similar
to how
Philippe Proulx [Sat, 29 Jun 2019 16:16:38 +0000 (12:16 -0400)]
test_{field,value}.py: test complex number RHS for binary operations
This patch adds injected tests for numeric field and value test cases in
which the RHS operand of the tested binary operations is a complex
number, both `-23+19j` and `0j`.
This is to ensure that a binary operator applied to a field object or a
value object and a Python complex number returns what's expected by the
langage, so you can do:
print(my_field + 8+4.2j)
if you wish.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I5cade062e285752d0a917a8b751824b0ebc97f16
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1587 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Philippe Proulx [Sat, 29 Jun 2019 15:28:34 +0000 (11:28 -0400)]
test_{field,value}.py: add comments
Those files contain code which is very similar. It should be a single
helper base class eventually, but this patch only adds comments to both
files to clarify the usage of some classes and methods.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I15d8d4e1d8d3444491b9f01deb6d2cdcc5d406d1
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1584 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 05:16:21 +0000 (01:16 -0400)]
bt2: create_value(): check `numbers` and `collections.abc` types
This is just like what's done within _NumericValue._extract_value(),
_IntegerValue._value_to_int(), RealValue._value_to_float(),
StringValue._value_to_str(), ArrayValue.__eq__(), and MapValue.__eq__().
It is more versatile than checking for the specific built-in types.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I93902e58b6adf3f9cd5c03c7c422148d577b5f0f
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1571 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Be more strict in the _value_to_int() functions: require that the
parameter's type is an instance of `numbers.Integral` instead of
`numbers.Real` to make the following raise a type error:
my_int_field.value = 17.5
I believe it's better to be strict here than to arbitrarily choose to
use the parameter's value's floor.
RealValue._value_to_float() already requires a real value, so this is
just analogous.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I3b981e90f7e2c133fa4d56e79333c4fe600eca8b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1569 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Those operators do a binary operation to a field object and another
operand, and then set the result as the field object's value.
Now that I think about it, it is weird to do something like:
my_int_field += 17.5
and that, when the value of `my_int_field` was 23, its value after the
operation becomes 40. This is not a problem for a native Python object
because `a` does not name the same object after the operation:
but we can't do this with our field object: this is internal Python's
magic.
So I believe we should be more strict. But this means you can't do:
my_int_field += 17.5
anymore (a `TypeError` exception would be raised).
This means a lot of specific automatically injected tests must
conditionally exist (depending on whether or not the test is possible
with the two operands, whereas it's always possible now).
Instead of doing this work, I choose to completely remove the support
for "self" operators. We can add useful ones in the future when we have
time, but I don't even think they are super useful: because a field
object becomes conceptually frozen as soon as you emit a message which
owns it indirectly, there's no point in doing `+=`, `-=`, `**=`, `<<=`,
and so on. For those rare cases, you can always do:
my_int_field = my_int_field + 17
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I5410ff8fdb2c3a1910e788542376d521601fdf8f
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1568 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Be more strict in _IntegerValue._value_to_int(): require that the
parameter's type is an instance of `numbers.Integral` instead of
`numbers.Real` to make the following raise a type error:
bt2.UnsignedIntegerValue(17.5)
and
a = bt2.UnsignedIntegerValue()
a.value = 17.5
I believe it's better to be strict here than to arbitrarily choose to
use the parameter's value's floor.
BoolValue._value_to_bool() and RealValue._value_to_float() already
require resp. a boolean value and a real value, so this is just
analogous.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I374baf8c5d69400b687daabea873598eb2543a47
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1567 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
but we can't do this with our value object: this is internal Python's
magic.
So I believe we should be more strict. But this means you can't do:
a = bt2.UnsignedIntegerValue(23)
a += 17.5
anymore (a `TypeError` exception would be raised).
This means a lot of specific automatically injected tests must
conditionally exist (depending on whether or not the test is possible
with the two operands, whereas it's always possible now).
Instead of doing this work, I choose to completely remove the support
for "self" operators. We can add useful ones in the future when we have
time, but I don't even think they are super useful: because a value
object becomes conceptually frozen as soon as you use it somehow,
there's no point in doing `+=`, `-=`, `**=`, `<<=`, and so on. For those
rare cases, you can always do:
a = bt2.UnsignedIntegerValue(23)
a = a + 17
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Icd5f0a9d0232c2f7ba91133561235fec7526083b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1566 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 03:54:08 +0000 (23:54 -0400)]
bt2: field.py: refactor field comparison
Changed:
* In _NumericField._extract_value(), do not check the `False`, and
`True` special cases: check that the parameter is an instance of the
`bool` type to return a boolean object.
* In _ArrayField.__eq__(), be more strict: expect that the parameter is
a sequence object, not just an iterable object. Before this, it was
possible to compare an array field to an ordered dict with keys equal
to the array field content, and this seems wrong as:
* In _StructureField.__eq__(), be more strict: expect that the parameter
is a mapping object, not just an iterable and indexable object. The
reason is similar to the _ArrayField.__eq__() case above. This should
be enough to compare to another structure field or to a dict (or
ordered dict).
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I17f33c24e9dea526e59b5058235d57facb51cfbf
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1564 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 03:51:24 +0000 (23:51 -0400)]
bt2: value.py: refactor value comparison
Changed:
* Remove _spec_eq() methods: each class implements its own __eq__()
method directly.
* Do not use native_bt.value_compare(): we never reached that, because
container value classes (`ArrayValue` and `MapValue`) implement their
own rich, recursive comparison.
* In _NumericValue._extract_value(), do not check the `_NumericValue`,
`False`, and `True` special case: check for `BoolValue` and `bool` to
return a boolean object in those cases.
* In NumericValue.__lt__() and NumericValue.__eq__(), do not check that
the parameter is a number object: self._extract_value() does this
already.
* In BoolValue._value_to_bool(), return the boolean value directly,
not using int(): it's already an integral number.
* In ArrayValue.__eq__(), be more strict: expect that the parameter is
a sequence object, not just an iterable object. Before this, it was
possible to compare an array value to an ordered dict with keys equal
to the array value content, and this seems wrong as:
* In MapValue.__eq__(), be more strict: expect that the parameter is a
mapping object, not just an iterable and indexable object. The reason
is similar to the ArrayValue.__eq__() case above. This should be
enough to compare to another map value or to a dict (or ordered dict).
Simon Marchi [Thu, 27 Jun 2019 03:33:58 +0000 (23:33 -0400)]
bt2: let Python message iterators implement seek beginning
This patch adds the possibility for message iterators implemented in
Python to implement the "can seek beginning" and "seek beginning"
operations.
A message iterator can support "seek beginning" by defining a
_seek_beginning method:
class MyIter(bt2._UserMessageIterator):
def _seek_beginning(self):
# Do the needful.
It can support the "can seek beginning" operation by defining a
_can_seek_beginning property, which must evaluate to a bool:
class MyIter(bt2._UserMessageIterator):
@property
def _can_seek_beginning(self):
# Do the needful, including returning a bool.
The behavior of the "can seek beginning" operation is made to mimic
the C API:
- If the iterator has a _can_seek_beginning attribute, it is used to
determine whether the iterator can seek beginning.
- Otherwise, the presence or absence of a _seek_beginning method
indicates whether it can.
Change-Id: I68c48b5cd30090bff833529cda9fa918c7e72b0b Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1555 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Philippe Proulx [Fri, 28 Jun 2019 21:03:27 +0000 (17:03 -0400)]
lib: internal: message.h: require logging
This patch makes the internal `message.h` require what it needs instead
of using the macros only if they exist. This makes it impossible to skip
logging in this header.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I2dd300931479a1d0752b6f90079c826324e19ce1
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1581 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 21:01:51 +0000 (17:01 -0400)]
lib: internal: object-pool.h: require logging
This patch makes the internal `object-pool.h` require what it needs
instead of using the macros only if they exist. This makes it impossible
to skip logging in this header.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I5f1b5550dfca7c7f3e8d62134a2917b059e4da0c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1580 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
This patch makes the internal `clock-snapshot-set.h` require what it
needs instead of using the macros only if they exist. This makes it
impossible to skip logging in this header.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ib8b3def2869ab214a3d38e3a1cea7b9954fc056c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1578 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 20:52:42 +0000 (16:52 -0400)]
lib: internal: graph.h: require logging and BT_ASSERT_{PRE,POST}()
This patch makes the internal `graph.h` require what it needs instead of
using the macros only if they exist. This makes it impossible to skip
logging or precondition/postcondition validation in this header.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I157809be7b56720b2f4a241c45491413f5b8366d
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1577 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 20:45:01 +0000 (16:45 -0400)]
lib: internal: define `BT_ASSERT_{PRE,POST}_SUPPORTED` in headers
This is to make some headers require that precondition and postcondition
assertion macros exist when including them, just like
`src/lib/logging.h` defines `BT_LIB_LOG_SUPPORTED` for the same reason.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ia2db609ae540b29129b9260c237f8b8d9657d977
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1576 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 28 Jun 2019 19:55:56 +0000 (15:55 -0400)]
lib: internal: add BT_ASSERT_POST()
BT_ASSERT_POST() is just like BT_ASSERT_PRE(), but it is used to
validate the returned values of user functions and methods.
The only difference with BT_ASSERT_PRE() is that its error message
indicates that a postcondition was not satisfied instead of a
precondition. This is simply more accurate.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I91815301ada19b42ba58f32db7b9c412cbe641e0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1574 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Thu, 27 Jun 2019 16:24:23 +0000 (12:24 -0400)]
Add `src/py-common`, containing bt_py_common_format_exception() for now
This new convenience library is meant to contain functions used by
Python bindings and the Python plugin provider.
As of this patch, it only offers bt_py_common_format_exception(), which
is similar to what bt2_py_loge_exception() used to do in
`native_bt_component_class.i`, except that it returns the formatted
exception string instead of logging it directly. Now
bt2_py_loge_exception() uses bt_py_common_format_exception().
bt_py_common_format_exception() will be needed in a future patch to set
the message of an error cause when appending from the Python plugin
provider.
Simon Marchi [Mon, 17 Jun 2019 23:18:13 +0000 (19:18 -0400)]
lib: validate monotonicity of messages from upstream component
This patch adds two checks after having called an iterator's next
method, to verify the validity of what is returned.
Clock class compatibility
-------------------------
Check that the clock classes of new streams created through this
iterator are compatible with the clock classes of streams previously
created through the same iterator.
The first time a stream is created through an iterator, we save some
properties of that clock class (whether its origin is the Unix epoch,
and if it isn't, its uuid). When new streams appear, we compare those
properties to make sure the clock classes of the new streams are
compatible with those properties.
Clock snapshots increase
------------------------
Check that the clock snapshots of all messages (that have one) don't go
back in time.
For each incoming message, if it has a clock snapshot, we compare it
with the clock snapshot of the previous message and assert that it isn't
lower (the "previous message time" is initialized to INT64_MIN).
Some messages don't have a clock snapshot, for those we do nothing.
I am not considering the "infinity" value that stream activity end
messages can have. It would add a bit more complexity to validate them
propertly, because it's possible to have something like this:
As you can see, it is possible to have a finite value (10) on stream 2
after having received an SAE message at infinity on stream 1. We could,
validate that nothing with a finite clock snapshot comes on stream 1
specifically. It would however require some per-stream data, which adds
a bit of complexity. And since we plan on removing these messages
anyway, it doesn't seem to be worth the effort.
Change-Id: If18f475ed056fda77a539d700e6ad310111f6d6b Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1486 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 27 Jun 2019 19:56:28 +0000 (15:56 -0400)]
Fix: bt2: fix reference counting of messages returned by Python components
The refcount of messages returned by components implemented in Python
(in _UserMessageIterator._next_from_native) is not right.
When the C iterator method "next" puts bt_message pointers in the output
array, it gives its reference to the bt_messages to the array. If it
wishes to keep owning a copy of the message, it needs to acquire a new
reference.
For this reason, we currently make the Message Python object release its
reference to the bt_message in _UserMessageIterator.__next__ (otherwise
there would be a double free). Releasing the reference has the effect
of setting the _ptr property to None, without changing the refcount.
However, it's possible for the user to keep a reference to that Python
object and re-use it. This is a problem, since the Message Python
object now unexpectedly no longer wraps a valid bt_message. Trying to
return that message in the following call to __next__, for example, will
fail.
To fix this, this patch makes _next_from_native acquire a new reference
before returning the bt_message. If the user didn't keep a reference to
the Python object, the Python object will die and put the bt_message.
The message array will therefore be the sole owner of the message. If
the user did keep a reference to the Python object, then both the Python
object and the message array will now own a reference to the message.
Change-Id: I6f7c472fddeea35509ee669e11d913bec7091f40 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1559 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 27 Jun 2019 20:03:55 +0000 (16:03 -0400)]
lib: fix compilation for glib < 2.40
Code from commit "lib: add logic in auto-seek to recreate stream state"
uses the return value of function g_hash_table_insert to validate that
the inserted key did not previously exist in the hash table. This
feature was introduced in glib 2.40, before that the function returned
void.
To avoid breaking compatibility with glib < 2.40, we can do a lookup
before inserting to achieve the same result.
Change-Id: Ibd5f58fb6ab8f083a3152105db9e05522893132c Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1560 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Michael Jeanson [Mon, 17 Jun 2019 20:48:02 +0000 (16:48 -0400)]
tests: Rework test suite
* Test script are not generated by autoconf anymore, instead the
parameters are passed trought a set of environment variable prefixed
with BT_TESTS_. This will enable the test suite to be used against
a system installed version of babeltrace with minimal configuration.
* Move all duplicated shell script code to 'utils.sh'.
* Add 'set -u' to common.sh to catch undefined variables in test
scripts. Variables that are expected to be undefined must be used as
'${var:-}'.
* All shell script is shellcheck tested.
* Removed unused test 'test_trace_ir'
* The python test runner now returns an error when no python test
scripts are found.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Change-Id: I15dcda1d147c8c72c6e4ee00052591eb6f9e30bd
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1429 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
After further discussions, we realized that there was a better way to
address the issue that commit 7bb4180 is fixing.
The real issue was that we were unconditionally converting the `other`
object to the `complex` type even when neither objects were of the
`complex` type. This was causing problem because of the lost of
precision when storing very large integer value in a variable of
`complex` type.
The better solution is to convert both objects to the type of the wider
object, and that is what Python does automatically. To enable that, we
simply need to convert the `other` object to it's corresponding standard
numeric type [1]. This commit uses the existing `_extract_value()`
method to achieve that.
Fix: bt2: erroneous integer comparison of Field and Value
Issue
=====
The `{Unsigned, Signed}IntegerValue` class inherit its __eq__() method
from the `_NumericValue` class. This method converts the `other` object
to a `complex` number to make the comparison as generic as possible. The
issue is that the large complex number comparison is unreliable as it's
using float comparison[1]. The timestamps we are dealing with will often
be going beyond the limit of exact comparison possible with the IEEE 754
double-precision[2].
I encountered this issue while comparing a
`bt2.value.SignedIntegerValue` to a large Python Integer representing a
timestamp in nanosecond.
Here is a showcase of the issue reproduced without the `bt2` bindings:
In [1]: 42 == complex(42)
Out[1]: True
The same issue is reproducible with the `_{Unsigned,
Signed}IntegerField`.
Solution
========
Implement the __eq__() and __lt__() methods in the `_IntegralValue`
class and but still use `_NumericValue`'s methods if the `other` object
is not an instance of `numbers.Integral`.
Implement the __eq__() and __lt__() methods in the `_IntegralField`
class and but still use `_NumericField`'s methods if the `other` object
is not an instance of `numbers.Integral`.
Drawback
========
None.
Note
====
I added test cases to cover large integer comparison for both Integer Value and
integer Field classes.
bt2: remove __le__() method already provided by @total_ordering
The `@total_ordering` decorator already provide all the rich comparison
methods because we implement the __lt__() and __eq__() methods. So there
is no need to implement any other rich comparison methods.
I chose __lt__() arbitrarily but it could have been __le__(), __gt__(),
or __ge__().
Documentation on `@total_ordering` [1]:
The class must define one of __lt__(), __le__(), __gt__(), or
__ge__(). In addition, the class should supply an __eq__() method.
Simon Marchi [Mon, 17 Jun 2019 16:58:31 +0000 (12:58 -0400)]
lib: add logic in auto-seek to recreate stream state
The trimmer component uses the seek_ns_from_origin iterator method to
seek its input iterator to the beginning of its trim window. After,
that, it's possible for the first message after the seek time to be, for
example, an event message. If so, it needs to re-create some messages
(possibly stream begin, stream activity begin and packet begin) to put
the stream in the right state. Otherwise, it would emit messages in an
invalid sequence.
A problem arises however with the following sequence:
1. The trim window starts at timestamp 50
2. Trimmer gets event message with timestamp 100 on stream 0. The
trimmer will send the following messages: stream 0 beginning, stream
0 activity beginning at timestamp 50, packet beginning message on
stream 0 at timestamp 50, event message on stream 0 at timestamp 100.
3. Trimmer gets event message with timestamp 120 on stream 1. The
trimmer will send the following messages: stream 1 beginning, stream
1 activity beginning at timestamp 50, packet beginning message on
stream 0 at timestamp 50, event message on stream 1 at timestamp 120.
The "stream 1 activity beginning at timestamp 50" message is invalid,
because it is earlier than the previously sent message "event message on
stream 0 at timestamp 100".
To avoid this going back in time, we need to emit the made up messages
first thing after seeking, instead of waiting for an event message (or
some other message requiring a stream and/or packet to be open) to emit
them. The trimmer can't do that itself, because it doesn't know the
state of the streams right after seeking (it doesn't know what messages
would have come before the seek point).
Therefore, the patch moves the functionality of emitting the missing
messages to the iterator code. When seeking (and using the go to
beginning then fast forward strategy, AKA auto-seek), the iterator code
goes through (and drops) all messages before the seek point. It can
therefore maintain a table of the state of all the streams. Once it has
reached the seek time, it can generate the messages required to bring
the streams in the state they are supposed to be, before transmitting
the messages coming from upstream.
Moving this functionality to the iterator also has the advantage that if
other components use the seek_ns_from_origin method of the iterator,
they'll receive a valid flow of messages. They won't need to do the
same complex work that the trimmer was doing to try re-create messages.
Since the trimmer can now assume a valid message flow from its input
iterator, it doesn't need anymore to generate its own stream beginning,
stream activity beginning and packet beginning messages before
transmitting a message that is supposed to come after one of these.
This means that the ensure_stream_state_is_inited and
ensure_cur_packet_exists functions can be removed.
Change-Id: I548f6ceafa5ad315c65be48077898595882e4511 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1450
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 17 Jun 2019 17:01:03 +0000 (13:01 -0400)]
iterator: save original next callback in iterator struct
When we restore the iterator's next method (after we overrode it with
post_auto_seek_next), we inspect the upstream component class to find
out what was the original method. It seems simpler to save and restore
the function pointer. This is what this patch does.
No user-visible changes intended.
Change-Id: If5eb7df692d2aa745f7f19e22c8046afb5b8d1d2 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1485 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Jonathan Rajotte [Wed, 19 Jun 2019 17:27:34 +0000 (13:27 -0400)]
tests: remove carriage return when using diff
Carriage return are a problem when running tests on cross execution
platform such as Windows. This only happen when diffing text generated
on Windows and text from a linux file.
The removal of mid-sentence '\r' is not a problem here since we do not
expect bt to generate this kind of string.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Change-Id: I107c5487bf3aa0f5cbd1fdfd1e948ee93f830d2f
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1521 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
Jonathan Rajotte [Wed, 19 Jun 2019 17:27:21 +0000 (13:27 -0400)]
libs: Do not rely on nftw for detecting the validity of plugin path
test_plugin.c:test_create_all_from_dir with non existing path fails on
cygwin. An error is expected but bt_plugin_find_all_from_dir returns
success.
The cygwin implementation of nftw is not posix compliant regarding
returning ENOENT on non-existent path. [1] [2]
The cygwin implementation of nftw does not return ENOENT on non-existent
path. Instead, the non-existent path is passed through the fn function
(nftw_append_all_from_dir) and the flag is set to FTW_NS.
Since we are "discovering" plugins, nftw_append_all_from_dir does not
error out on FTW_NS and simply skip the file.
The proposed solution is to stat() the provided path and validate that
no errors occur before using nftw().
Simon Marchi [Thu, 27 Jun 2019 00:04:49 +0000 (20:04 -0400)]
cli: add missing line in --plugin-path help
When doing "babeltrace2 convert --help", a line is missing from the
--plugin-path option help. This patch adds it, copied from the help of
the other commands.
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I823385cae29afa2bfa0650d4c5dd0142dd4f9cea
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1549 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 26 Jun 2019 13:49:57 +0000 (09:49 -0400)]
lib: reverse order of bt_self_component_port_input_message_iterator::auto_seek_msgs queue
The messages in this queue are in reverse chronological order, meaning
we push the incoming messages to the head and later consume them from
the tail.
It would be more expected (although functionally equivalent) to do the
opposite, so that the messages are in chronological order when iterating
the list in forward.
So this patch just swaps the push/pop calls around.
Change-Id: Id4892e12e064376e919647c0d8bb838156ef0fc5 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1543
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Fri, 21 Jun 2019 22:37:24 +0000 (18:37 -0400)]
bt2: replace copy of headers for SWIG with includes
The current approach of writing SWIG interface files is to copy paste
declarations. This can be tedious and error prone, when the API is
modified.
This patch changes the interface files to include the Babeltrace header
files containing those declarations instead.
The main difficulty with this is that we can't rely on parameter names
for typemap matching anymore. In most cases, using just the type is
fine (e.g. the only reason a function takes a
bt_self_component_port_input ** is as an output parameter).
In the case of "user data" it's a bit more tricky, since the type is
"void *". In these cases (in native_bt_component.i and
native_bt_port.i), I made it so we clear the typemap at the end of the
file, so hopefully it doesn't apply to an unexpected function in another
file.
projects / babeltrace.git / log
