[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [help-GIFT] gift-write-feature-descs segfaults... (is the GIFT broke

From: David Squire
Subject: Re: [help-GIFT] gift-write-feature-descs segfaults... (is the GIFT broken?)
Date: Sun, 27 Aug 2006 17:51:04 +0100
User-agent: Thunderbird (Macintosh/20060719)

Jonas Lindqvist wrote:
This is undoubtedly my fault, for not checking the other binaries.

I wouldn't call this anyone's "fault". I expected the compiler to help, and was surprised that it didn't.

There is a general lesson here. Before committing any patches, a full build of the whole system, and test cases run (e.g. adding a collection and testing that it can be queried).

Why not instead see if we can get the compiler/linker to help out finding missing functions? Adding a collecting and then querying it, will test just the simple adding and querying in the test. Nothing more. It's difficult to test all different paths of execution possible, although testing some paths is better than nesting none. (Would an indexing-run followed by some queries assert that all the different libs work as intended?)
I was not intending to suggest run-time tests as a replacement for compile-time error catches. But I still think that a full build and test should be done before a commit... of course we should have a "make test" for this.

It is always hard (even impossible) to guarantee that all execution paths are exercised, but we do have a pretty good idea of the main ones:

add a collection
get a random set of images from that collection
query the collection for images similar to some query image

For example, using the latest CVS version of the GIFT, I was able to run gift-add-collection on a collection, and query it for a random set of images. No problem. However, whenever asking for any similar images, it failed.

Well of course I meant an actual similarity query. From my point of view getting random images is not really a query (though that is how it appears in MRML and the code). Originally (before MRML) I did this in the interface, not by querying.

I'm sure some other modification could break a not-so-often used functions while still appearing to work when running some basic "normal" tests. By using the compiler/linker to alert us, we would be able to find these errors in compile time instead of in runtime.
Much nicer in my opinion.

One other tool would be grep. If I decide to alter the signature of a function/method, then grep-ing for that name in the source-files, could lead me to places where that function is used. Only... this is not convenient in this case since the GIFT source code is so full of unused junk that isn't compiled anyway. (Is gabor981029.c an example of primitive source control?) I get too many matches in files that aren't needed.

Yep. It's ancient history. The feature extraction stuff was not added to GIFT by me (though it's my fault it didn't get cleaned up before being added). So, the FeatureExtraction directory was originally just a copy of a directory of mine that included all sorts of stuff - old versions, test code, functions written for general use with RGB/HSV, etc. There's no reason not to clean it up.

My current feeling is that the currently evolving optimized version of "GIFT features 1.0" should stay in the trunk, and if I (or others) get around to writing other feature extractors that do different things, they should be available not as replacements but as alternatives, perhaps selected via an optional argument to

Yes, normally C screams and yells at you, but the gift was written
without function prototypes, so the compiler cant tell, until link time.

Well, it didn't even complain when linking...
-Wmissing-prototypes could be used to at least get a compiler warning, but I'm not sure why the linker didn't speak up.

Perhaps dynamic linking? Just guessing here.

So, at some point a decision was made to do without the prototypes. In my original version (from Viper, before it was bundled with the GIFT), that starts:
#include "extract_features.proto"

I would guess that that changed because someone did not have access to cproto...
 ...include all necessary header-files, the old-fashioned way...
Maybe we should fix that now? Or is there a reason not to?

Not that I can think of.



Dr David McG. Squire, Senior Lecturer, on sabbatical in 2006
Caulfield School of Information Technology, Monash University, Australia
CRICOS Provider No. 00008C

reply via email to

[Prev in Thread] Current Thread [Next in Thread]