Bug 146502

Summary: d2asc segfaults with more than 40 vars, and should also offer to read them from a file
Product: [Applications] kst Reporter: Nicolas Brisset <nicolas.brisset>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: crash    
Priority: NOR    
Version First Reported In: 1.3.1   
Target Milestone: ---   
Platform: unspecified   
OS: Solaris   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description Nicolas Brisset 2007-06-07 17:44:08 UTC 
Version:           1.3.1 (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

I saved one of my colleagues' life today with d2asc :-) but he has to to a lot of tedious script manipulations because he needs more than 40 variables from the data file.
Looking at the source code, I see than 40 is a hardcoded value for the number of fields and I understand why it crashes :-( It would be nice to read all variable names passed (to the limit of the command line length!), or even better allow to read them from a file.
Comment 1 Andrew Walker 2007-06-07 21:09:34 UTC 
SVN commit 672647 by arwalker:

BUG:146502 Remove arbitrary limits on number of vectors and length of filename to prevent crashes

 M  +30 -26    d2asc.cpp  


--- branches/work/kst/1.5/kst/src/d2asc/d2asc.cpp #672646:672647
@@ -15,7 +15,7 @@
  *                                                                         *
  ***************************************************************************/
 
-#include <stdlib.h> // atoi
+#include <stdlib.h>
 #include <kconfig.h>
 #include <kinstance.h>
 
@@ -26,6 +26,11 @@
 #include "kstdataobjectcollection.h"
 #undef protected
 
+struct fieldEntry {
+  QString field;
+  bool doHex;
+};
+
 void Usage() {
   fprintf(stderr, "usage: d2asc filename [-f <first frame>]\n");
   fprintf(stderr, "             [-n <numframes>] [-s skip [-a]] \n");
@@ -44,28 +49,26 @@
   atexit(exitHelper);
   KInstance inst("d2asc");
   KstDataSourcePtr file;
-  int i;
 
   KConfig *kConfigObject = new KConfig("kstdatarc", false, false);
   KstDataSource::setupOnStartup(kConfigObject);
 
-  char field_list[40][120], filename[180];
-  bool do_hex[40];
-  int n_field=0;
+  fieldEntry field;
+  QValueList<fieldEntry> fieldList;
+  char *filename;
+  bool do_ave = false, do_skip = false;
   int start_frame=0, n_frames=2000000;
-  bool do_ave = false, do_skip = false;
   int n_skip = 0;
   int NS=0, i_S;
+  int i;
 
   if (argc < 3 || argv[1][0] == '-') {
     Usage();
     return -1;
   }
 
-  for (i = 0; i < 40; i++)
-    do_hex[i] = false;
+  filename = argv[1];
 
-  strcpy(filename, argv[1]);
   for (i = 2; i < argc; i++) {
     if (argv[i][0] == '-') {
       if (argv[i][1] == 'f') {
@@ -82,19 +85,22 @@
         do_ave = true;
       } else if (argv[i][1] == 'x') {
         i++;
-        strcpy(field_list[n_field], argv[i]);
-        do_hex[n_field] = true;
-        n_field++;
+        field.field = argv[i];
+        field.doHex = true;
+        fieldList.append(field);
       } else {
         Usage();
       }
     } else {
-      strcpy(field_list[n_field], argv[i]);
-      n_field++;
+      field.field = argv[i];
+      field.doHex = false;
+      fieldList.append(field);
     }
   }
 
-  if (!do_skip) do_ave = false;
+  if (!do_skip) {
+    do_ave = false;
+  }
 
   file = KstDataSource::loadSource(filename);
   if (!file || !file->isValid() || file->isEmpty()) {
@@ -104,30 +110,28 @@
   /** make vectors and fill the list **/
   QPtrList<KstRVector> vlist;
 
-  for (i=0; i<n_field; i++) {
-
-    if (!file->isValidField(field_list[i])) {
+  for (i=0; i<int(fieldList.size()); i++) {
+    if (!file->isValidField(fieldList[i].field)) {
       fprintf(stderr, "d2asc error: field %s in file %s is not valid\n",
-              field_list[i], filename);
+              fieldList[i].field.latin1(), filename);
       return -3;
     }
-    KstRVectorPtr v = new KstRVector(file, field_list[i], KstObjectTag("tag", KstObjectTag::globalTagContext), start_frame, n_frames, n_skip, n_skip>0, do_ave);
+    KstRVectorPtr v = new KstRVector(file, fieldList[i].field, KstObjectTag("tag", KstObjectTag::globalTagContext), start_frame, n_frames, n_skip, n_skip>0, do_ave);
     vlist.append(v);
   }
 
   /* find NS */
-  for (i = 0; i < n_field; i++) {
-
+  for (i = 0; i < int(fieldList.size()); i++) {
     while (vlist.at(i)->update(-1) != KstObject::NO_CHANGE)
       ; // read vector
-
-    if (vlist.at(i)->length() > NS)
+    if (vlist.at(i)->length() > NS) {
       NS = vlist.at(i)->length();
+    }
   }
 
   for (i_S = 0; i_S < NS; i_S++) {
-    for (i = 0; i < n_field; i++) {
-      if (do_hex[i]) {
+    for (i = 0; i < int(fieldList.size()); i++) {
+      if (fieldList[i].doHex) {
         printf("%4x ",  (int)vlist.at(i)->interpolate(i_S, NS));
       } else {
         printf("%.16g ", vlist.at(i)->interpolate(i_S, NS));
Comment 2 George Staikos 2007-06-08 02:30:31 UTC 
Looks good to me.

On 8-Jun-07, at 3:32 AM, Barth Netterfield wrote:

> Upon review, this should be back-ported to 1.4.1
>
> cbn
>
> On June 7, 2007, Andrew Walker wrote:
>> ------- You are receiving this mail because: -------
>> You are the assignee for the bug, or are watching the assignee.
>>
>> http://bugs.kde.org/show_bug.cgi?id=146502
>> arwalker sumusltd com changed:
>>
>>            What    |Removed                     |Added
>> --------------------------------------------------------------------- 
>> ------
>> - Status|NEW                         |RESOLVED
>>          Resolution|                            |FIXED
>>
>>
>>
>> ------- Additional Comments From arwalker sumusltd com  2007-06-07  
>> 21:09
>> ------- SVN commit 672647 by arwalker:
>>
>> BUG:146502 Remove arbitrary limits on number of vectors and length of
>> filename to prevent crashes
>>
>>  M  +30 -26    d2asc.cpp
>>
>>
>> --- branches/work/kst/1.5/kst/src/d2asc/d2asc.cpp #672646:672647
>>  @ -15,7 +15,7  @
>>   *
>> *
>> ********************************************************************* 
>> ******
>> /
>>
>> -#include <stdlib.h> // atoi
>> +#include <stdlib.h>
>>  #include <kconfig.h>
>>  #include <kinstance.h>
>>
>>  @ -26,6 +26,11  @
>>  #include "kstdataobjectcollection.h"
>>  #undef protected
>>
>> +struct fieldEntry {
>> +  QString field;
>> +  bool doHex;
>> +};
>> +
>>  void Usage() {
>>    fprintf(stderr, "usage: d2asc filename [-f <first frame>]\n");
>>    fprintf(stderr, "             [-n <numframes>] [-s skip [-a]]  
>> \n");
>>  @ -44,28 +49,26  @
>>    atexit(exitHelper);
>>    KInstance inst("d2asc");
>>    KstDataSourcePtr file;
>> -  int i;
>>
>>    KConfig *kConfigObject = new KConfig("kstdatarc", false, false);
>>    KstDataSource::setupOnStartup(kConfigObject);
>>
>> -  char field_list[40][120], filename[180];
>> -  bool do_hex[40];
>> -  int n_field=0;
>> +  fieldEntry field;
>> +  QValueList<fieldEntry> fieldList;
>> +  char *filename;
>> +  bool do_ave = false, do_skip = false;
>>    int start_frame=0, n_frames=2000000;
>> -  bool do_ave = false, do_skip = false;
>>    int n_skip = 0;
>>    int NS=0, i_S;
>> +  int i;
>>
>>    if (argc < 3 || argv[1][0] == '-') {
>>      Usage();
>>      return -1;
>>    }
>>
>> -  for (i = 0; i < 40; i++)
>> -    do_hex[i] = false;
>> +  filename = argv[1];
>>
>> -  strcpy(filename, argv[1]);
>>    for (i = 2; i < argc; i++) {
>>      if (argv[i][0] == '-') {
>>        if (argv[i][1] == 'f') {
>>  @ -82,19 +85,22  @
>>          do_ave = true;
>>        } else if (argv[i][1] == 'x') {
>>          i++;
>> -        strcpy(field_list[n_field], argv[i]);
>> -        do_hex[n_field] = true;
>> -        n_field++;
>> +        field.field = argv[i];
>> +        field.doHex = true;
>> +        fieldList.append(field);
>>        } else {
>>          Usage();
>>        }
>>      } else {
>> -      strcpy(field_list[n_field], argv[i]);
>> -      n_field++;
>> +      field.field = argv[i];
>> +      field.doHex = false;
>> +      fieldList.append(field);
>>      }
>>    }
>>
>> -  if (!do_skip) do_ave = false;
>> +  if (!do_skip) {
>> +    do_ave = false;
>> +  }
>>
>>    file = KstDataSource::loadSource(filename);
>>    if (!file || !file->isValid() || file->isEmpty()) {
>>  @ -104,30 +110,28  @
>>    /** make vectors and fill the list **/
>>    QPtrList<KstRVector> vlist;
>>
>> -  for (i=0; i<n_field; i++) {
>> -
>> -    if (!file->isValidField(field_list[i])) {
>> +  for (i=0; i<int(fieldList.size()); i++) {
>> +    if (!file->isValidField(fieldList[i].field)) {
>>        fprintf(stderr, "d2asc error: field %s in file %s is not  
>> valid\n",
>> -              field_list[i], filename);
>> +              fieldList[i].field.latin1(), filename);
>>        return -3;
>>      }
>> -    KstRVectorPtr v = new KstRVector(file, field_list[i],
>> KstObjectTag("tag", KstObjectTag::globalTagContext), start_frame,  
>> n_frames,
>> n_skip, n_skip>0, do_ave); +    KstRVectorPtr v = new KstRVector 
>> (file,
>> fieldList[i].field, KstObjectTag("tag",  
>> KstObjectTag::globalTagContext),
>> start_frame, n_frames, n_skip, n_skip>0, do_ave); vlist.append(v);
>>    }
>>
>>    /* find NS */
>> -  for (i = 0; i < n_field; i++) {
>> -
>> +  for (i = 0; i < int(fieldList.size()); i++) {
>>      while (vlist.at(i)->update(-1) != KstObject::NO_CHANGE)
>>        ; // read vector
>> -
>> -    if (vlist.at(i)->length() > NS)
>> +    if (vlist.at(i)->length() > NS) {
>>        NS = vlist.at(i)->length();
>> +    }
>>    }
>>
>>    for (i_S = 0; i_S < NS; i_S++) {
>> -    for (i = 0; i < n_field; i++) {
>> -      if (do_hex[i]) {
>> +    for (i = 0; i < int(fieldList.size()); i++) {
>> +      if (fieldList[i].doHex) {
>>          printf("%4x ",  (int)vlist.at(i)->interpolate(i_S, NS));
>>        } else {
>>          printf("%.16g ", vlist.at(i)->interpolate(i_S, NS));
>> _______________________________________________
>> Kst mailing list
>> Kst@kde.org
>> https://mail.kde.org/mailman/listinfo/kst
>
>
> _______________________________________________
> Kst mailing list
> Kst@kde.org
> https://mail.kde.org/mailman/listinfo/kst


--
George Staikos
KDE Developer				http://www.kde.org/
Staikos Computing Services Inc.		http://www.staikos.net/