[Fontconfig] Code review needed ,spotted by Coverity

[Patrick Lam]

Frederic Crozat wrote:
> Le mercredi 12 avril 2006 à 01:44 -0400, Patrick Lam a écrit :
> 
>>Frederic Crozat wrote:
> 
> 
>>>-defect #759 in fccharset.c / FcCharSetSubtractCount :
>>>*bm might be NULL because of assignment to bi.leaf->map and then it is
>>>accessed without any NULL test. I don't know if bi.leaf->map is never
>>>NULL.
>>
>>I don't understand this code yet.  The problem is not that ->map is 
>>NULL, but that bi might be NULL.  ->map can't be null, it's a 
>>FcChar32[256/32].
> 
> 
> Well, I didn't understand it either. But you are right about the
> incorrect access, the defect is about bi being NULL.

I believe that it's safe, although I'm only 95% sure.

>>>-defects #783, #784, #785, #786 : 
>>>* if config->maxObjects == 0, but config->substPattern or
>>>config->substFont are not NULL, st, while NULL, will be accessed
>>>* at line 1497, there is a test against thisValue being NULL (so, it
>>>might be NULL), but FcConfigDel called at line 1506 might deferences
>>>thisValue, causing a crash.
>>>* at line 1463, l might be leaked if switch (e->op) is handled by
>>>default case). I don't know if it is possible.
>>
>>Can you give more details on these defects?
> 
> 
> Here are the url for the defects, they seems to work, I don't know for
> how long. Otherwise, I'll copy the defect page elsewhere.
> 
> http://scan.coverity.com:7468/view-error.cgi?id=7529&runid=19&user=fcrozat&magic=636dfd390b4c5cc1f308d2841e3a3c5e&prevpage=query-run-table.cgi%3Ffile%3Dfontconfig%26magic%3D636dfd390b4c5cc1f308d2841e3a3c5e%26prevpage%3D%252Findex.cgi%253Fmagic%253D636dfd390b4c5cc1f308d2841e3a3c5e%2526user%253Dfcrozat%26runid%3D19%26user%3Dfcrozat&prevpagename=Search%20Results

The (hidden) invariant here is that config->maxObjects bounds from above 
the number of possible iterations that the for loop may take.  If 
maxObjects is 0, the loop body is never executed, so st is never 
dereferenced.

> http://scan.coverity.com:7468/view-error.cgi?id=10169&runid=21&user=fcrozat&magic=636dfd390b4c5cc1f308d2841e3a3c5e&prevpage=query-run-table.cgi%3Ffile%3Dfontconfig%26magic%3D636dfd390b4c5cc1f308d2841e3a3c5e%26prevpage%3D%252Findex.cgi%253Fmagic%253D636dfd390b4c5cc1f308d2841e3a3c5e%2526user%253Dfcrozat%26runid%3D21%26user%3Dfcrozat&prevpagename=Search%20Results

Looks plausible; I've put in a test for nullness.

> http://scan.coverity.com:7468/view-error.cgi?id=10470&runid=21&user=fcrozat&magic=636dfd390b4c5cc1f308d2841e3a3c5e&prevpage=query-run-table.cgi%3Ffile%3Dfontconfig%26magic%3D636dfd390b4c5cc1f308d2841e3a3c5e%26prevpage%3D%252Findex.cgi%253Fmagic%253D636dfd390b4c5cc1f308d2841e3a3c5e%2526user%253Dfcrozat%26runid%3D21%26user%3Dfcrozat&prevpagename=Search%20Results

> http://scan.coverity.com:7468/view-error.cgi?id=10469&runid=21&user=fcrozat&magic=636dfd390b4c5cc1f308d2841e3a3c5e&prevpage=query-run-table.cgi%3Ffile%3Dfontconfig%26magic%3D636dfd390b4c5cc1f308d2841e3a3c5e%26prevpage%3D%252Findex.cgi%253Fmagic%253D636dfd390b4c5cc1f308d2841e3a3c5e%2526user%253Dfcrozat%26runid%3D21%26user%3Dfcrozat&prevpagename=Search%20Results

Should be freed in the default case, which I've changed.  That should 
fix both of these.

> There are two new defects for some cases which were not completely fixed
> with yesterday patches.
> 
> First one is in fcfreetype.c, some deadcode was still there. Based on
> bedhad comment ("if you ask me, you should remove both goto Fail's in
> that loop. it's trying to list scripts supported by the font, and if one
> of the subtables is missing, we can simply ignore that script, instead
> of failing the entire operation), I've cooked one patch which remove two
> of the goto to fix defect #2088.

I'll take behdad's word for it...

> The other defect is a memory leak in FcPatternFreeze (fcpat.c) in error
> case (defect #2089).

Fixed; committing shortly.

pat