Bug 118838

Summary:	[patch] getdata: assertion failed on zero read in lincom and multiply
Product:	[Applications] kst	Reporter:	D. V. Wiebe <dvw>
Component:	general	Assignee:	kst
Status:	RESOLVED FIXED
Severity:	crash
Priority:	NOR
Version:	1.x
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	Patch for bug 118838

Description D. V. Wiebe 2005-12-22 04:33:40 UTC

Version:           1.2.0_devel (using KDE KDE 3.4.2)
Installed from:    Compiled From Sources
Compiler:          gcc (GCC) 3.4.4 
OS:                Linux

The supplied patch fixes a failed assertion in AllocTmpbuff when the read for the first field in a lincom or multiply returns zero frames.

In this case, the unpatched getdata attempts then to allocate a temporary buffer for the second field in the lincom or multiply of length zero, resulting in the failed assertion.

This patch resolves this issue by returning immediately (thus skipping the attempted read of the second field), and reporting to the caller that zero frames have been read.  I'm not quite sure that this is the right response in this situation, in light of George's comments on bug 112762 where he says:

>> KstRVector expects that it can read one sample from a frame, however.  This means that n_read is 1 and n_read2 is 0, so n_read is adjusted to 0.  This is wrong.  n_read should be 1 still, and Kst keeps trying to read this sample over and over.

That gives me the impression kst assumes it can always read at least one sample, and so getdata returning zero frames read might cause it to be unhappy.  However, I can't see how getdata could legitimately return anything else but zero in this case seeing as it hasn't actually read any data.

This bug was encountered with the BLAST map maker (which uses getdata pared out of the kst source tree) when it attempted to read a field which an overworked NFS server had temporarily caused to go away, so the zero read for the field was unexpected and transitory.

This patch depends on the patch provided in bug 118837.

Comment 1 D. V. Wiebe 2005-12-22 04:34:28 UTC

Created attachment 14018 [details]
Patch for bug 118838

Comment 2 George Staikos 2005-12-22 05:08:05 UTC

SVN commit 490483 by staikos:

fix memory leak and assertion failure.  Patches from Don
BUGS: 118837, 118838


 M  +21 -4     getdata.c  


--- trunk/extragear/graphics/kst/kst/datasources/dirfile/getdata.c #490482:490483
@@ -1201,7 +1201,14 @@
       error_code);
 
   recurse_level--;
-  if (*error_code != GD_E_OK) return(1);
+
+  if (*error_code != GD_E_OK)
+    return(1);
+
+  /* Nothing to lincomise */
+  if (*n_read == 0)
+    return 1;
+  
   ScaleData(data_out, return_type, *n_read, L->m[0], L->b[0]);
 
   if (L->n_infields > 1) {
@@ -1230,8 +1237,10 @@
           return_type, tmpbuf,
           error_code);
       recurse_level--;
-      if (*error_code != GD_E_OK)
+      if (*error_code != GD_E_OK) {
+        free(tmpbuf);
         return(1);
+      }
 
       ScaleData(tmpbuf, return_type, n_read2, L->m[i], L->b[i]);
 
@@ -1290,8 +1299,14 @@
       error_code);
 
   recurse_level--;
-  if (*error_code != GD_E_OK) return(1);
 
+  if (*error_code != GD_E_OK)
+    return 1;
+
+  /* Nothing to multiply */
+  if (*n_read == 0)
+    return 1;
+
   recurse_level++;
 
   /* find the samples per frame of the second field */
@@ -1316,8 +1331,10 @@
       return_type, tmpbuf,
       error_code);
   recurse_level--;
-  if (*error_code != GD_E_OK)
+  if (*error_code != GD_E_OK) {
+    free(tmpbuf);
     return(1);
+  }
 
   if (n_read2 > 0 && n_read2 * spf1 < *n_read * spf2) {
     *n_read = n_read2 * spf1 / spf2;