Bug 116981

Summary:	[patch] getdata: allow inclusion of other files in a dirfile format file
Product:	[Applications] kst	Reporter:	D. V. Wiebe <dvw>
Component:	general	Assignee:	kst
Status:	RESOLVED FIXED
Severity:	wishlist
Priority:	NOR
Version:	1.x
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
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.