Bug 116981

Summary: [patch] getdata: allow inclusion of other files in a dirfile format file
Product: [Applications] kst Reporter: D. V. Wiebe <dvw>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: wishlist    
Priority: NOR    
Version: 1.x   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Patch to allow getdata INCLUDE capabilities in dirfile format files
Patch to allow getdata INCLUDE capabilities in dirfile format files
File descriptor leak patch

Description D. V. Wiebe 2005-11-24 01:09:17 UTC 
Version:           1.2.0dr1 (using KDE KDE 3.4.2)
Installed from:    Compiled From Sources
Compiler:          gcc (GCC) 3.3.6 
OS:                Linux

The supplied patch (generated against 1.2.0dr1) gives getdata the ability to "include" additional format files in the primary format file of a dirfile.  The syntax is:

INCLUDE <file>

Included files are parsed as if their contents were pasted verbatim into the format file at the location of the INCLUDE.  Inclusion is recursive and while parsing a particular dirfile, getdata will remember which files it has already read, so it won't crash and burn on encountering a circular inclusion.

If an included format file cannot be opened (because, say, it doesn't exist), format file parsing will fail, and getdata will return (the new) error code GD_E_OPEN_INCLUDE.

Matt orgininally came up with the idea and, after both Enzo and Barth expressed enthusiasm for it, I went ahead and implemented it.  With BLAST analysis, we're drowing in dirfile fields and format file entries.  This allows us to better maintain our working dirfiles when dealing with dirfile fragments generated by various people working on the analysis, since it removes the necessity to maintain a single, monolithic format file.

The patch also seals a small file descriptor leak by closing format files after reading them, rather than keeping them open forever.

(This implementation is less cool that my original idea, which was to have getdata pipe the format file through "gcc -E" before parsing it... ;-)
Comment 1 D. V. Wiebe 2005-11-24 01:10:10 UTC 
Created attachment 13627 [details]
Patch to allow getdata INCLUDE capabilities in dirfile format files
Comment 2 George Staikos 2005-11-24 01:30:06 UTC 
The descriptor leak fix should definitely go in, but I'd rather leave the 
feature until after 1.2.0.  Is that OK with you guys?  We need fewer changes 
in trunk now...
Comment 3 D. V. Wiebe 2005-11-24 01:44:49 UTC 
How far away is 1.2.0?

Personally, I'm content either way.  I'll happily patch this stuff in myself as needed until it gets into TRUNK.
Comment 4 D. V. Wiebe 2005-11-24 01:54:58 UTC 
Created attachment 13628 [details]
Patch to allow getdata INCLUDE capabilities in dirfile format files

Sorry, I just noticed I uploaded the wrong version of the patch.  This is the
correct one.
Comment 5 Matthew Truch 2005-11-24 02:14:58 UTC 
On Thu, Nov 24, 2005 at 12:30:07AM -0000, George Staikos wrote:
> The descriptor leak fix should definitely go in, but I'd rather leave the 
> feature until after 1.2.0.  Is that OK with you guys?  We need fewer changes 
> in trunk now...


I understand not immediatly, but would it be able to go in to something
like 1.2.1?  I have a feeling that 1.3.0 is going to be a while off.
Comment 6 George Staikos 2005-11-24 02:29:01 UTC 
> 01:44 ------- How far away is 1.2.0?


  Less than a month I hope.  We just need to clean up scripting, view objects, 
general bugfix, and hopefully get the docs updated.

  If you extract the file descriptor leak fix I'll apply it.
Comment 7 George Staikos 2005-11-24 02:32:13 UTC 
> I understand not immediatly, but would it be able to go in to something
> like 1.2.1?  I have a feeling that 1.3.0 is going to be a while off.


  I think so.
Comment 8 D. V. Wiebe 2005-11-24 03:16:42 UTC 
Created attachment 13629 [details]
File descriptor leak patch

> If you extract the file descriptor leak fix I'll apply it. 

Here it is.
Comment 9 George Staikos 2005-11-24 03:41:06 UTC 
SVN commit 482757 by staikos:

Don's patch to fix descriptor leak fix
CCBUG: 116981


 M  +7 -0      getdata.c  


--- trunk/extragear/graphics/kst/kst/datasources/dirfile/getdata.c #482756:482757
@@ -345,12 +345,14 @@
       *error_code = GD_E_FORMAT;
       FreeF(F);
       Formats.n--;
+      fclose(fp);
       return(NULL);
     }
     if (strlen(in_cols[0])>FIELD_LENGTH) {
       *error_code = GD_E_FIELD;
       FreeF(F);
       Formats.n--;
+      fclose(fp);
       return(NULL);
     }
     if (strcmp(in_cols[1], "RAW")==0) {
@@ -398,14 +400,19 @@
       FreeF(F);
       Formats.n--;
       *error_code = GD_E_FORMAT;
+      fclose(fp);
       return(NULL);
     }
     if (*error_code!=GD_E_OK) {
       FreeF(F);
       Formats.n--;
+      fclose(fp);
       return(NULL);
     }
   }
+
+  fclose(fp);
+  
   /** Now sort the lists */
   if (F->n_raw > 1) {
     for (i=0; i<F->n_raw; i++) {
Comment 10 Matthew Truch 2006-01-06 19:29:08 UTC 
George added these features with svn commit 494718.