Re: [Tracker] PATCH: Fix segfault in tracker-ontologies (shows in tracker-miner-fs) when ontologies are missing

[Jonatan Pålsson <jonatan palsson pelagicore com>]

Hi!

On 9 August 2013 16:40, Martyn Russell <martyn lanedo com> wrote:
> On 08/08/13 16:27, Philip Van Hoof wrote:
> > Ho Jonatan,
>
>
> Hi Philip, Jonatan,
>
>
> > I don't really agree with this approach. We should not 'just' check for
> > NULL and let the software continue as if nothing happens. Invalid
> > ontologies means that we can't go on. So abort() is probably the only
> > sensible way out.
>
>
> Yes, I agree. This is on the same level as corrupt database type errors, i.e
> we can not operate at all without a minimal requirement. One of those
> requirements is a fully functioning and properly existing ontology.

I see. Hm. I have noticed that the store continues to run in other
cases, such as when properties are not found (try changing
nao:lastModified to nao:lastModifiedd), or when declaring new
properties (probably the wrong terminology..) stemming from
non-existing classes (try xsd:string a rdfs:Classs .).

Perhaps some check should be performed when loading the ontologies to
see that all is well? Is there a zero-acceptance policy on brokenness
in the ontologies? If so, I suppose it is easier to perform this
check, and abort if anything at all is broken.

> > Trying to accept that stuff returns NULL and then happily continuing the
> > rest of the software isn't really an option. Either the software can do
> > its stuff and then it does it, or it can't and then it aborts (with an
> > appropriate error message and/or error mechanism). Trying to continue
> > and by that further destroying data and/or corrupt databases is not good.
>
>
> Exactly.

Yep. Good point. I agree fully.

> > In this case what could also be done, if you do want to improve
> > error-handling, is that tracker-store returns an error for each and
> > every of its D-Bus calls. I don't consider trying to handle NULL an
> > improvement (rather, it's worse that way as you know about the problem
> > later in the code rather than sooner this way).
>
>
> Agree with Philip here. Much better to have if (!foo) { g_error ("Holy cow,
> where's my brain? Please find my ontology"); }
>
> :)
>
> The alternative is, you investigate some other problem later on and find
> this is the point of inception all along.

Yes, I agree with you both. We shouldn't continue with broken
ontologies. I noticed that the gvdb reader behaves the same (gives a
NULL in the same location) when the gvdb files are corrupt. The header
of the gvdb appears to be checked for corruption, but the contents are
not checked in the same way. I modified the gvdb file on disk with a
text editor and achieved the same behavior.

I think it is not a bad idea to check for NULL here, alternatively
perform some integrity check of the file. Would you accept a patch
which replaces the warnings with g_error?

-- 
Regards,
Jonatan Pålsson

Pelagicore AB
Ekelundsgatan 4, 6th floor, SE-411 18 Gothenburg, Sweden