Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 323659

Summary: A poblem can happen with ResourceLookupTree adding resource approach
Product: [Tools] CDT Reporter: John Liu <john_ws_liu>
Component: cdt-coreAssignee: Markus Schorn <mschorn.eclipse>
Status: RESOLVED FIXED QA Contact: Doug Schaefer <cdtdoug>
Severity: normal    
Priority: P3    
Version: 7.0   
Target Milestone: 7.0.1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
a proposed patch applied to cdt_7_0 branch
none
more elegant solution mschorn.eclipse: iplog-

Description John Liu CLA 2010-08-25 17:19:49 EDT 
Build Identifier: 

ResourceLookupTree uses an inverse file extensions list to do a reverse extension search and decide which file should be added into the tree for future looking up. See Function private void add(IResource res).

The inverse extension list is initialized by taking entire platform content types excluding the cdt registered content types. See function initFileExtensions()

A problem can happen with this approach if someone extends the cdt and needs to define a new content type with C like extension again. Then the C like extensions (i.e .c) will be added into the inversion extension list. As a consequence, any file with C like extensions redefined by the new content type will not added to ResourceLookupTree. This will cause bugs for any function which needs ResourceLookupTree  to look for a resource, for example ErrorParserManager's function findFileName(String)



Reproducible: Always
Comment 1 John Liu CLA 2010-08-25 17:31:45 EDT 
Created attachment 177477 [details]
a proposed patch applied to cdt_7_0 branch

Propose a fix patch for discussion. 

The fix just adds an extension relevant check to cdt default extension list(fDefaultExtensions) for the incoming resource in the function:

    private void add(IResource res)

Actually do we still need to check the inverse extension list fCurrentExtensions? I believe the inverse extension list is much bigger than the default extension list. So why we pick a longer list to search?
Comment 2 Markus Schorn CLA 2010-08-26 03:37:48 EDT 
Created attachment 177499 [details]
more elegant solution

(In reply to comment #1)
> The fix just adds an extension relevant check to cdt default extension
> list(fDefaultExtensions) for the incoming resource ...
A more elegant solution is to make sure that the default-extensions are part
of the cdt-project-extensions, see my patch.

> Actually do we still need to check the inverse extension list
> fCurrentExtensions? I believe the inverse extension list is much bigger than
> the default extension list. So why we pick a longer list to search?
The inverse list is motivated by the fact that in c/c++ you can include files with any extension or files without extension. Therefore in a CDT project the relevant files are the ones that are known c/c++ files (*.c, ...) plus the ones that have an unknown or no extension.

If I read the source code correctly, the problem can occur only when you have a content type that does not derive from one of the cdt-content types and contains *.c as a file pattern. Is that actually the case?
Comment 3 John Liu CLA 2010-08-26 10:40:24 EDT 
(In reply to comment #2)
> Created an attachment (id=177499) [details]
> more elegant solution
> (In reply to comment #1)
> > The fix just adds an extension relevant check to cdt default extension
> > list(fDefaultExtensions) for the incoming resource ...
> A more elegant solution is to make sure that the default-extensions are part
> of the cdt-project-extensions, see my patch.
> > Actually do we still need to check the inverse extension list
> > fCurrentExtensions? I believe the inverse extension list is much bigger than
> > the default extension list. So why we pick a longer list to search?
> The inverse list is motivated by the fact that in c/c++ you can include files
> with any extension or files without extension. Therefore in a CDT project the
> relevant files are the ones that are known c/c++ files (*.c, ...) plus the ones
> that have an unknown or no extension.
> If I read the source code correctly, the problem can occur only when you have a
> content type that does not derive from one of the cdt-content types and
> contains *.c as a file pattern. Is that actually the case?
Thanks, Markus for your quick fix.

Yes. That is the case when another content type also has C like extensions. I agree your fix is more closing to the original logic and it works too. 

When can you check in your patch to cdt_7_0 stream and head?
Comment 4 Markus Schorn CLA 2010-08-26 11:14:54 EDT 
(In reply to comment #3)
> Yes. That is the case when another content type also has C like extensions. 
ok.

> ...
> When can you check in your patch to cdt_7_0 stream and head?

Fixed in 7.0.1 and 8.0 > 20100826.