zip_append_analog() does not free most of the memory it allocates. Address
this by moving all sanity checks that do not rely on anything else at the
beginning of the function before any allocations are done. And then make
sure to properly free all allocated memory on all paths leaving the
function.
srzip: Fix handling of analog channels with index 0
0 is a valid index for a channel. Using it as the value for the terminating
entry of analog_index_map causes zip_append_analog() to falsely assume that
no channel was found when a packet for a channel with index 0 was received.
This prevents the data for the channel to be added to the sigrok session
file.
Instead use -1, which is not a valid channel index, as
the terminating entry value.
Marvin Schmidt [Fri, 13 May 2016 10:27:14 +0000 (12:27 +0200)]
build: Replace AX_CXX_COMPILE_STDCXX_11 with latest AX_CXX_COMPILE_STDCXX
The former appended the necessary switch to enable C++11 to the CXXFLAGS
whereas AX_CXX_COMPILE_STDCXX appends it to CXX which has the benefit
that all C++ sources are compiled using the same C++ standard. Therefore
it is no longer necessary to manually hardcode '-std=c++11' anywhere
like we did in the Ruby bindings linker command and assures that the
compilation of them is done with C++11 support as well.
Marvin Schmidt [Fri, 13 May 2016 15:44:12 +0000 (17:44 +0200)]
build: Fix distribution of Ruby bindings
The bindings file was not listed in EXTRA_DIST and therefore not
distributed. We also need to provide an target to uninstall the Ruby
bindings and add it to UNINSTALL_EXTRA in order to make `make distcheck`
happy.
Now that the signature of std_init() matches that of the driver init()
callback we can remove all wrapper functions around std_init() and use it
directly as the init() callback.
std_init(): Drop check if pass in driver is non-NULL
std_init() checks if the pass in struct sr_dev_driver is non-NULL and
prints a error message and returns an error if it is NULL.
std_init() is exclusively called from driver init() callbacks for which the
core already checks if the struct sr_dev_driver is non-NULL before invoking
the callback. This means the check in std_init() will always evaluate to
false. So drop this check.
This also means that the prefix parameter that was used in the error
message is no longer needed and can be removed from the function signature.
Doing so will make the std_init() function signature identical to the
init() callback signature which will allow to directly use it as such.
Match std_init() parameter order to the driver init() callback
The std_init() callback has the order of the first two paramters opposite
to the init() callback. This is primarily due to historical development.
Since the std_init() function is usually called from a driver's init()
callback aligning the order will allow direct register pass through rather
than having to swap them around. It also allow to eventually use the
std_init() function directly as the init() callback.
The zeroplus-logic-cube driver uses libusb_get_device_list() but neglects
to call the matching libusb_device_list_free() on the error path. This will
leak the memory allocated for the list as well as all the devices.
To address the issue use sr_usb_open() instead of open-coding its
functionality. sr_usb_open() correctly handles freeing the device list.
The issue was discovered using the following coccinelle semantic patch:
// <smpl>
@@
identifier devlist;
expression ctx, ret;
statement S;
@@
(
libusb_get_device_list(ctx, &devlist);
|
ret = libusb_get_device_list(ctx, &devlist);
if (ret < 0) S
)
... when != libusb_free_device_list(devlist, ...)
*return ...;
// </smpl>
The victor-dmm driver uses libusb_get_device_list() but neglects to call
the matching libusb_device_list_free() on the error path of libusb_open().
This will leak the memory allocated for the list as well as all the
devices.
To address the issue use sr_usb_open() instead of open-coding its
functionality. sr_usb_open() correctly handles freeing the device list.
The issue was discovered using the following coccinelle semantic patch:
// <smpl>
@@
identifier devlist;
expression ctx, ret;
statement S;
@@
(
libusb_get_device_list(ctx, &devlist);
|
ret = libusb_get_device_list(ctx, &devlist);
if (ret < 0) S
)
... when != libusb_free_device_list(devlist, ...)
*return ...;
// </smpl>
The testo driver uses libusb_get_device_list() but neglects to call the
matching libusb_device_list_free() on the error path. This will leak the
memory allocated for the list as well as all the devices.
To address the issue use sr_usb_open() instead of open-coding its
functionality. sr_usb_open() correctly handles freeing the device list.
The issue was discovered using the following coccinelle semantic patch:
// <smpl>
@@
identifier devlist;
expression ctx, ret;
statement S;
@@
(
libusb_get_device_list(ctx, &devlist);
|
ret = libusb_get_device_list(ctx, &devlist);
if (ret < 0) S
)
... when != libusb_free_device_list(devlist, ...)
*return ...;
// </smpl>
lascar-el-usb: lascar_scan(): Fix USB device list leak
lascar_scan() calls libusb_get_device_list() but never the matching
libusb_free_device_list(). This will leak the memory allocated for the
device list as well as all the devices. To fix this add the missing
libusb_free_device_list().
While we are at it also make sure to handle errors returned by
libusb_get_device_list().
The issue was discovered using the following coccinelle semantic patch:
// <smpl>
@@
identifier devlist;
expression ctx, ret;
statement S;
@@
(
libusb_get_device_list(ctx, &devlist);
|
ret = libusb_get_device_list(ctx, &devlist);
if (ret < 0) S
)
... when != libusb_free_device_list(devlist, ...)
*return ...;
// </smpl>
Some drivers check in some of their driver callbacks if the driver has been
initialized and return an error if it has not.
For the scan() callback the sigrok core checks if the driver has been
initialized and if not returns an error. So it is not possible that the
scan() callback gets called if the driver is not initialized. Without the
scan() callback succeeding it is not possible to get a reference to a
device which is associated with the driver, so it is not possible that any
of the device specific callbacks is called without the driver first being
initialized either.
In conclusion these checks are not necessary since they never evaluate to
true and can be dropped. If they should ever become necessary they should
be done in the sigrok core so all drivers and all callbacks are equally
handled.
Drop SR_CONF_SET flag from SR_CONF_CONTINUOUS options
SR_CONF_CONTINUOUS is a capability option indicating whether a device
supports continuous capture or not. If the option exists the device
supports continuous capture and otherwise it doesn't. There is no value
associated with it and hence setting the SR_CONF_SET flag is nonsensical.
None of the drivers which set SR_CONF_SET for SR_CONF_CONTINUOUS handle it
in their config_set() callback and return an error if an application tried
to perform a config_set() operation for SR_CONF_CONTINUOUS.
Simply remove the SR_CONF_SET flag from all SR_CONF_CONTINUOUS options.
Introduce standard implementation of the dev_list() callback
Every single hardware driver has the very same implementation of the
dev_list() callback. Put this into a helper function in the standard helper
library and use it throughout the drivers. This reduces boiler-plate code
by quite a bit.
If a driver does not implement a dev_clear() callback the core will
automatically call std_dev_clear(di, NULL). Remove all driver dev_clear()
implementations that are identical to default. This reduces the amount of
boiler-plate code.
std_init() allocates a drv_context struct which needs to be freed by the
driver in its cleanup struct. But the vast majority of drivers does never
does this causing memory leaks.
Instead of addressing the issue by manually adding code to free the struct
to each driver introduce a new helper function std_cleanup() that takes
care of this. In addition to freeing the drv_context struct std_cleanup()
also invokes sr_dev_clear() which takes care of freeing all devices
attached to the driver.
Combining both operations in the same helper function allows to use
std_cleanup() as the cleanup callback for all existing drivers, which
reduces the amount of boiler-plate code quite a bit.
All drivers are updated to use the new helper function.
Devices for the scpi-pps driver do have additional data attached to it that
needs to be freed when the device is freed. While the driver gets it right
for the cleanup() callback it does not for the dev_clear() callback. This
will cause memory leaks when sr_dev_clear() is called for this driver.
To fix this let the dev_clear() free the additional data.
Devices for the deree-de5000 driver do have additional data attached to it
that needs to be freed when the device is freed. While the driver gets it
right for the cleanup() callback it does not implement a dev_clear()
callback, so the default dev_clear() implementation is used which will not
free the additional data. This will cause memory leaks when sr_dev_clear()
is called for this driver.
To fix this provide a dev_clear() implementation that frees the additional
data.
Devices for the demo driver do have additional data attached to it that
needs to be freed when the device is freed. While the driver gets it right
for the cleanup() callback it does not implement a dev_clear() callback, so
the default dev_clear() implementation is used which will not free the
additional data. This will cause memory leaks when sr_dev_clear() is called
for this driver.
To fix this provide a dev_clear() implementation that frees the additional
data.
baylibre-acme: Properly free GErrors returned by g_file_get_contents()
When g_file_get_contents() encounters an error a new GError will be
allocated and passed back to the application. The application is
responsible for freeing this GError.
The baylibre-acme driver currently does not do this and as a result leaks
memory during the scan process when no device is found.
Add the missing g_error_free() invocations to fix the issue.
Make sure to free the FTDI device list and the FTDI context in scan_all()
otherwise memory leaks can be observed. Also make sure to free the FTDI
context in scan_device() on the error path.
Uwe Hermann [Sat, 26 Mar 2016 18:45:04 +0000 (19:45 +0100)]
Factor out std_session_send_df_end() helper.
This makes the code shorter, simpler and more consistent, and also
ensures that the (same) debug messages are always emitted and the
packet.payload field is consistently set to NULL always, etc.
Joel Holdsworth [Sun, 24 Apr 2016 18:38:12 +0000 (20:38 +0200)]
fx2lafw: Add CTL2 clocking command flag to header
The USBee AX hardware needs a sampling clock that is lower than
the 30MHz or 48MHz that the FX2 has to offer. This flag will enable
clocking via the CTL2 pin that is an even divisor of the main clock.
hp-3457a: Implement AC, ACDC, and four-wire resistance modes
The driver did not look at the mq_flags provided with the
SR_CONF_MEASURED_QUANTITY key, and it defaulted to DC measurements.
Use the second member of the tuple provided by the config key, which
represents the flags for the measurement, and set the instrument's
measurement mode accordingly.
On the high-end bench multimeters, resistance can be measured with a
kelvin connection as well as the more common two wire method. Provide
a flag which can indicate if four-wire mode is used.
hp-3457a: Do not retrigger new measurement after the last sample
Due to a PEBKAC error, after the last sample was sent, a new
measurement was triggered, but its value was never sent down the
session bus. This is easily fixed by incrementing devc->num_samples
right after a measurement is sent instead of when a measurement is
retriggered.
Benjamin Larsson [Tue, 12 Apr 2016 18:25:01 +0000 (20:25 +0200)]
memory leak fix: g_variant_print() usage fix
g_variant_print() allocates memory during call. Save the pointer
so that it can be free'd afterwards.
==10048== 16 bytes in 1 blocks are definitely lost in loss record 17 of 37
==10048== at 0x4C2DEAE: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10048== by 0x536C85D: g_realloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x53877C6: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x5388B60: g_string_append_vprintf (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x5388D83: g_string_append_printf (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x539D034: g_variant_print_string (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x539C92C: g_variant_print (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x4E5F713: log_key (hwdriver.c:597)
==10048== by 0x4E5FCD5: sr_config_set (hwdriver.c:752)
==10048== by 0x408C1A: run_session (session.c:661)
==10048== by 0x404FC5: main (main.c:267)
==10048==
==10048== 16 bytes in 1 blocks are definitely lost in loss record 18 of 37
==10048== at 0x4C2DEAE: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10048== by 0x536C85D: g_realloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x53877C6: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x5388B60: g_string_append_vprintf (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x5388D83: g_string_append_printf (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x539D034: g_variant_print_string (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x539C92C: g_variant_print (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4400.1)
==10048== by 0x4E5F713: log_key (hwdriver.c:597)
==10048== by 0x4E5FBDD: sr_config_get (hwdriver.c:709)
==10048== by 0x4080D7: datafeed_in (session.c:196)
==10048== by 0x4E5D47E: sr_session_send (session.c:1192)
==10048== by 0x4E62682: std_session_send_df_header (std.c:101)
Markus Siegert [Mon, 11 Apr 2016 19:54:10 +0000 (21:54 +0200)]
hantek-6xxx: Use power of 2 usb packet sizes
Using non power of 2 sizes causes the driver to not work on OS X.
Depending on the usb transfer mode the buffer sizes will map
perfectly to the underlying transport protocol.
Stefan Brüns [Sun, 17 Apr 2016 17:14:27 +0000 (19:14 +0200)]
hameg-hmo: Also start reading on timeout (workaround for USBTMC)
scpi_serial generate an POLLIN event after the requested data is returned
by the instrument. For USBTMC it is necessary to
1. send an REQUEST_DEV_DEP_MSG_IN request
2. submit an USB bulk read transfer asynchronously.
Using the synchronous libusb_bulk_read() does not generate an POLLIN event.
Solving this properly needs major surgery in spci_usbtmc_libusb.
Stefan Brüns [Sat, 16 Apr 2016 21:38:05 +0000 (23:38 +0200)]
spci: Terminate all commands with a linefeed for all transports
While some transports add a terminating (carriagereturn+)linefeed
unconditionally, the USBTMC transport does not. At least the R&S HMO1002
requires the linefeed and locks up otherwise. Fixes bug #784.
This changes the TCP and VXI transport from CR+LF to LF only.
Also fixes a possible memory leak for VXI, where the temporary command
buffer was not freed in case of a write error.
Stefan Brüns [Sat, 16 Apr 2016 21:37:40 +0000 (23:37 +0200)]
scpi/usbtmc: fix remote locking according to USBTMC spec
According to USBTMC usb488 subclass spec, wValue hast to be 0 for both
LOCAL_LOCKOUT and GO_TO_LOCAL. At least required for R&S HMO1002, the
bad request results in a STALL. Fixes bug #783.
Diego F. Asanza [Wed, 13 Apr 2016 18:50:37 +0000 (20:50 +0200)]
Ensure DSLogic can be stopped.
After acquisition start, DSLogic stores samples in memory, and when done it
sends a USB packet with the trigger position.
This initial fillup can take some time. If the user requests a session stop
in between, the USB transfer is cancelled and the session hangs because it
is not closed properly.
This commit manages this case and closes the session properly when
acquisition is stopped by the user.
projects / libsigrok.git / log