Hello Stefan,
thanks very much for reviewing the patch !
I have now improved it by following your comments, please see the new
version (v2) in the next message...
Regards,
Guido
On Mon, 29/10/2018 at 19.17 +0000, Brüns, Stefan wrote:
> On Montag, 29. Oktober 2018 17:53:19 CET Guido Trentalancia wrote:
> > Correctly set the length of the buffer used to hold the SCPI
> > response
> > from the device containing the binary acquisition data.
> >
> > Signed-off-by: Guido Trentalancia <gu...@trentalancia.com>
> > ---
> > src/scpi/scpi.c | 36 +++++++++++++++++-------------------
> > 1 file changed, 17 insertions(+), 19 deletions(-)
> >
> > diff -pru libsigrok-git-20102018-orig/src/scpi/scpi.c libsigrok-
> > git-
> > 20102018-fix-scpi-response-buffer-length/src/scpi/scpi.c
> > --- libsigrok-git-20102018-orig/src/scpi/scpi.c 2018-10-20
> > 13:12:30.983966965 +0200
> > +++ libsigrok-git-20102018-fix-scpi-response-buffer-
> > length/src/scpi/scpi.c 2018-10-29 17:45:33.251699570 +0100
> > @@ -974,14 +974,14 @@ SR_PRIV int sr_scpi_get_block(struct sr_
> > *scpi_response = NULL;
> >
> > /* Get (the first chunk of) the response. */
> > - while (response->len < 2) {
> > + do {
> > ret = scpi_read_response(scpi, response, timeout);
> > if (ret < 0) {
> > g_mutex_unlock(&scpi->scpi_mutex);
> > g_string_free(response, TRUE);
> > return ret;
> > }
> > - }
> > + } while (response->len < 2);
> >
> > /*
> > * SCPI protocol data blocks are preceeded with a length
> > spec.
> > @@ -1028,25 +1028,23 @@ SR_PRIV int sr_scpi_get_block(struct sr_
> > g_string_erase(response, 0, 2 + llen);
> >
> > /*
> > - * If the initially assumed length does not cover the data
> > block
> > - * length, then re-allocate the buffer size to the now
> > known
> > - * length, and keep reading more chunks of response data.
> > + * Re-allocate the buffer size to the now known length
> > + * and keep reading more chunks of response data.
> > */
> > - if (response->len < (unsigned long)(datalen)) {
> > - int oldlen = response->len;
> > - g_string_set_size(response, datalen);
> > - g_string_set_size(response, oldlen);
> > - }
> > + int oldlen = response->len;
> > + g_string_set_size(response, datalen);
>
> NAK!
>
> The above is a reservation call. If you skip the
> g_string_set_size(response,
> oldlen);, any new data will be appended at offset datalen, not
> oldlen.
>
>
> >
> > - while (response->len < (unsigned long)(datalen)) {
> > - ret = scpi_read_response(scpi, response, timeout);
> > - if (ret < 0) {
> > - g_mutex_unlock(&scpi->scpi_mutex);
> > - g_string_free(response, TRUE);
> > - return ret;
> > - }
> > - if (ret > 0)
> > - timeout = g_get_monotonic_time() + scpi-
> >
> > > read_timeout_us;
> >
> > + if (oldlen < datalen) {
> > + do {
> > + ret = scpi_read_response(scpi, response,
> > timeout);
> > + if (ret < 0) {
> > + g_mutex_unlock(&scpi->scpi_mutex);
> > + g_string_free(response, TRUE);
> > + return ret;
> > + }
> > + if (ret > 0)
> > + timeout = g_get_monotonic_time() +
> > scpi->read_timeout_us;
> > + } while (response->len < (unsigned
> > long)(datalen));
> > }
> >
> > g_mutex_unlock(&scpi->scpi_mutex);
> >
> >
> > _______________________________________________
> > sigrok-devel mailing list
> > sigrok-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/sigrok-devel
>
>
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
