Bug 635 - PulseView crashes while importing csv file.
Summary: PulseView crashes while importing csv file.
Status: RESOLVED FIXED
Alias: None
Product: libsigrok
Classification: Unclassified
Component: Input: csv (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: High major
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
: 817 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-08-26 15:43 CEST by Kubsztal
Modified: 2017-06-07 00:13 CEST (History)
6 users (show)



Attachments
csv file crashing PulseView (463 bytes, application/x-zip-compressed)
2015-08-26 15:43 CEST, Kubsztal
Details
Correct return value for csv::end module (637 bytes, patch)
2015-10-15 21:16 CEST, Pavlo Taranov
Details
Fix \r\n newline handling (706 bytes, patch)
2016-11-24 21:38 CET, Pinky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kubsztal 2015-08-26 15:43:10 CEST
Created attachment 153 [details]
csv file crashing PulseView

PulseView (version 0.3.0) crashes while importing csv file of size ~75kB (attached).
Comment 1 Kubsztal 2015-08-26 21:57:40 CEST
Under linux too:
terminate called after throwing an instance of 'sigrok::Error'
  what():  generic/unspecified error
Aborted
Comment 2 Uwe Hermann 2015-08-26 23:34:24 CEST
Confirmed on Linux (git HEAD):

(gdb) bt
#0  0x00007f9f63de0107 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007f9f63de14e8 in __GI_abort () at abort.c:89
#2  0x00007f9f6470a64d in __gnu_cxx::__verbose_terminate_handler() () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007f9f647086a6 in  () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007f9f647086f1 in  () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007f9f647324e8 in  () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007f9f66b9b0a4 in start_thread (arg=0x7f9f4fa61700) at pthread_create.c:309
#7  0x00007f9f63e9107d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111


Removing Windows category, this is OS-independent.
Comment 3 Pavlo Taranov 2015-10-15 21:16:45 CEST
Created attachment 168 [details]
Correct return value for csv::end module

Looks like there is error in libsigrok/src/input/csv.c in function 

static int end(struct sr_input *in)

sr_session_send called but return value is not checked. 

diff --git a/src/input/csv.c b/src/input/csv.c
index 063145a..528022c 100644
--- a/src/input/csv.c
+++ b/src/input/csv.c
@@ -760,7 +760,7 @@ static int end(struct sr_input *in)
        if (inc->started) {
                /* End of stream. */
                packet.type = SR_DF_END;
-               sr_session_send(in->sdi, &packet);
+        ret = sr_session_send(in->sdi, &packet);
        }
 
        return ret;

this means that call to sr_input_end(in); would return error, as ret variable was set to -1. sigrock-cli just ignore this statement, so it works there, but PV has un-catched exception.

Proposed patch will update value for ret variable, so no error code returned.
Comment 4 leonerd 2015-11-04 15:54:47 CET
+1 to this patch. I was was just having trouble loading CSVs and this solves it PV for me.
Comment 5 Pinky 2016-11-24 21:37:47 CET
The proposed patch is wrong, just hides the problem.

The exception is caused by trailing \n character left in input buffer after file processing when \r\n newlines are used.
Comment 6 Pinky 2016-11-24 21:38:58 CET
Created attachment 273 [details]
Fix \r\n newline handling
Comment 7 Gerhard Sittig 2017-06-05 17:35:54 CEST
Cause of the reported crash:

The attached CSV file is incomplete, the last line lacks the end-of-line 
encoding.  The current implementation of the CSV import insists in that 
end-of-line marker, and complains in its absence.  Then pulseview raises 
an internal error which terminates the program.

Workaround:

A quick workaround is to provide proper text files with complete lines 
that include the line terminator.

Alternative:

Adjustment of the CSV input module to weaken its constraint yet correctly 
process input data that is read in chunks and contains partial lines is more 
involved.
Comment 8 Uwe Hermann 2017-06-07 00:02:57 CEST
Fixed in 241c386a4f727faa7c8379ae9212661011aa50ae and 7f4c3a622405b409e579e8344192e8daf15cb817, thanks!
Comment 9 Uwe Hermann 2017-06-07 00:13:30 CEST
*** Bug 817 has been marked as a duplicate of this bug. ***