Fontconfig - Apr 2006 - Patch: fix double free (spotted by Coverity)

Hi,

I''m reviewing Coverity defect results on fontconfig
(http://scan.coverity.com/ ) to try to fix all defects for 2.3.95 (or at
least 2.4.0).

First patch fixes double free (and closing the same fd twice too).

I''ll send other patches if needed while reviewing defects list.
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-doublefree.patch
Type: text/x-patch
Size: 1471 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/d310c759/fontconfig-2.3.94-doublefree.bin

Yet another patch for Coverity defect :

this checks again null pattern (only in low memory case when malloc
returns NULL).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-nullpatterncheck.patch
Type: text/x-patch
Size: 1205 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/fd3d3ee6/fontconfig-2.3.94-nullpatterncheck.bin

This patch fixes memory leak in FcDirCacheWrite when hash collision
occurs (Coverity defect #1829)

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-hashmemleak.patch
Type: text/x-patch
Size: 1262 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/8c54405e/fontconfig-2.3.94-hashmemleak.bin

This patch fixes memory leak in FcCacheWrite in bail cases (Coverity
report #1828).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-cachememleak.patch
Type: text/x-patch
Size: 1018 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/d6adfe3d/fontconfig-2.3.94-cachememleak.bin

Le lundi 10 avril 2006 ? 15:33 +0200, Frederic Crozat a ?crit
:
> This patch fixes memory leak in FcCacheWrite in bail cases (Coverity
> report #1828).
My comment was incorrect, patch is fixing memleak in
FcConfigBuildFonts ;)

Patch with correct changelog attached.

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-cachememleak.patch
Type: text/x-patch
Size: 1019 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/05df88f0/fontconfig-2.3.94-cachememleak.bin

Another memleak fix in FcGlobalCacheLoad from Coverity reports
( FcStrSetAdd already copies its second argument, no need to pass a copy
of it) and this time, it is not a corner case ;)

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-filenameleak.patch
Type: text/x-patch
Size: 1140 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/0a6b7e8c/fontconfig-2.3.94-filenameleak.bin

The following patch fixes memory and file descriptor leaks (Coverity
reports #777 and #1826) in error cases for FcDirScanConfig.

Please review this patch carefully, as I used goto for bail conditions
and tried to call free functions only one time in the FcDirScanConfig
code (to ease code review in the future).
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcdirmemleaks.patch
Type: text/x-patch
Size: 2487 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/3055dcad/fontconfig-2.3.94-fcdirmemleaks.bin

Frederic Crozat wrote:
> The following patch fixes memory and file descriptor leaks (Coverity
> reports #777 and #1826) in error cases for FcDirScanConfig.
> 
> Please review this patch carefully, as I used goto for bail conditions
> and tried to call free functions only one time in the FcDirScanConfig
> code (to ease code review in the future).
I''ve committed the earlier patches, but not this one yet.

pat

Frederic Crozat wrote:
> The following patch fixes memory and file descriptor leaks (Coverity
> reports #777 and #1826) in error cases for FcDirScanConfig.
> 
> Please review this patch carefully, as I used goto for bail conditions
> and tried to call free functions only one time in the FcDirScanConfig
> code (to ease code review in the future).
I''ve committed a modified version of this patch.  I think it''s
correct.

pat

Le lundi 10 avril 2006 ? 11:46 -0400, Patrick Lam a ?crit
:
> Frederic Crozat wrote:
> > The following patch fixes memory and file descriptor leaks (Coverity
> > reports #777 and #1826) in error cases for FcDirScanConfig.
> > 
> > Please review this patch carefully, as I used goto for bail conditions
> > and tried to call free functions only one time in the FcDirScanConfig
> > code (to ease code review in the future).
> 
> I''ve committed the earlier patches, but not this one yet.
Great.

Here is another memleak fix for non-error case in FcGlobalCacheSave
(Coverity report #1825).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-headerleak.patch
Type: text/x-patch
Size: 1099 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/42b21234/fontconfig-2.3.94-headerleak.bin

Le lundi 10 avril 2006 ? 12:07 -0400, Patrick Lam a ?crit
:
> Frederic Crozat wrote:
> > The following patch fixes memory and file descriptor leaks (Coverity
> > reports #777 and #1826) in error cases for FcDirScanConfig.
> > 
> > Please review this patch carefully, as I used goto for bail conditions
> > and tried to call free functions only one time in the FcDirScanConfig
> > code (to ease code review in the future).
> 
> I''ve committed a modified version of this patch.  I think
it''s correct.
It looks ok to me.

To celebrate, here is another memleak fix (Coverity report #1824) for
FcDirCacheUnlink ;)

Don''t worry, there are only 28 defects left to check, I''ll
probably
continue tomorrow...

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-hashmemleak2.patch
Type: text/x-patch
Size: 1129 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/842963e6/fontconfig-2.3.94-hashmemleak2.bin

Ok, I swear, this is the last fix for today ;)

This patch fixes a memleak in FcConfigEvaluate with strings (Coverity
defect #1823).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fccfgmemleak.patch
Type: text/x-patch
Size: 1256 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060410/f9461124/fontconfig-2.3.94-fccfgmemleak.bin

Frederic Crozat wrote:
> Ok, I swear, this is the last fix for today ;)
> 
> This patch fixes a memleak in FcConfigEvaluate with strings (Coverity
> defect #1823).
Got it.

Thanks Dawson (Engler, a cofounder of Coverity.)

pat

Ok, second day of Coverity fixes.

First patch fixes three memleaks in fcpat.c (Coverity defect #1820,
#1821, #1822) when malloc fails.

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-patternmemleak.patch
Type: text/x-patch
Size: 1957 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/1dfebafc/fontconfig-2.3.94-patternmemleak.bin

This patch fixes multiple memory leaks in FcNameUnparseLangSet (Coverity
defect #1819).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fclangmemleak.patch
Type: text/x-patch
Size: 1079 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/1b7fad68/fontconfig-2.3.94-fclangmemleak.bin

This patch fixes a potential crash in fc-lang if parsed file syntax is
incorrect (Coverity defect #763).
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-includecrash.patch
Type: text/x-patch
Size: 1148 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/7af0c15f/fontconfig-2.3.94-includecrash.bin

The following patch prevent potential null pointer access in fc-cat
(Coverity defect #1804).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fccatnullfix.patch
Type: text/x-patch
Size: 1402 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/5b8cc0dd/fontconfig-2.3.94-fccatnullfix.bin

The following patch fixes two Coverity defects (#767 and #1195) in
fcfreetype.c.

One is cristal clear (access to pointer before NULL test), the other
part might be wrong : error was probably not assigned anymore in the
code (so testing its value was useless after line 2782), so I assumed
error retrieval was missing.
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcfreetypefixes.patch
Type: text/x-patch
Size: 1511 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/38d0f9ef/fontconfig-2.3.94-fcfreetypefixes-0001.bin

This patch remove dead code and memory leak from FcObjectUnserialize
(Coverity report #1194).

I think it is dead code since allocated variable is not used anywhere,
probably a leftover from previous version.
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcnamefix.patch
Type: text/x-patch
Size: 1231 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/ce88f41f/fontconfig-2.3.94-fcnamefix.bin

Coverity found two memory leaks in fccharset.c : FcCharSetPutLeaf (#1192
and #1193) in the "if (fcs->bank != FC_BANK_DYNAMIC)" TRUE branch :
leaves and numbers. But after reading this part of the code, I''m not
sure if it is doing anything at all : memory is allocated for leaves and
numbers but those variables are not used at all after "if () {} else
{}"
stuff, so they are leaked and not used at all..

Could somebody with much more knowledge of this code check it ?

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva

Frederic Crozat wrote:
> Coverity found two memory leaks in fccharset.c : FcCharSetPutLeaf (#1192
> and #1193) in the "if (fcs->bank != FC_BANK_DYNAMIC)" TRUE
branch :
> leaves and numbers. But after reading this part of the code, I''m
not
> sure if it is doing anything at all : memory is allocated for leaves and
> numbers but those variables are not used at all after "if () {} else
{}"
> stuff, so they are leaked and not used at all..
> 
> Could somebody with much more knowledge of this code check it ?
Oops!  I meant to do this:

Index: src/fccharset.c
==================================================================RCS file:
/cvs/fontconfig/fontconfig/src/fccharset.c,v
retrieving revision 1.25.4.13
diff -u -p -r1.25.4.13 fccharset.c
--- src/fccharset.c     7 Apr 2006 17:27:39 -0000       1.25.4.13
+++ src/fccharset.c     11 Apr 2006 14:18:55 -0000
@@ -168,6 +168,7 @@ FcCharSetPutLeaf (FcCharSet *fcs,
         return FcFalse;
      if (fcs->bank != FC_BANK_DYNAMIC)
      {
+        /* convert to dynamic */
         int i;

         leaves = malloc ((fcs->num + 1) * sizeof (FcCharLeaf *));
@@ -183,6 +184,10 @@ FcCharSetPutLeaf (FcCharSet        *fcs,
             leaves[i] = FcCharSetGetLeaf(fcs, i);
         memcpy (numbers, FcCharSetGetNumbers(fcs),
                 fcs->num * sizeof (FcChar16));
+
+       fcs->bank = FC_BANK_DYNAMIC;
+       fcs->u.dyn.leaves = leaves;
+       fcs->u.dyn.numbers = numbers;
      }
      else
      {

I''ve committed all of your patches as well as this one.

pat

Le mardi 11 avril 2006 ? 10:21 -0400, Patrick Lam a ?crit
:
> Frederic Crozat wrote:
> > Coverity found two memory leaks in fccharset.c : FcCharSetPutLeaf
(#1192
> > and #1193) in the "if (fcs->bank != FC_BANK_DYNAMIC)"
TRUE branch :
> > leaves and numbers. But after reading this part of the code,
I''m not
> > sure if it is doing anything at all : memory is allocated for leaves
and
> > numbers but those variables are not used at all after "if () {}
else {}"
> > stuff, so they are leaked and not used at all..
> > 
> > Could somebody with much more knowledge of this code check it ?
> 
> Oops!  I meant to do this:
> 
> Index: src/fccharset.c
> ==================================================================> RCS
file: /cvs/fontconfig/fontconfig/src/fccharset.c,v
> retrieving revision 1.25.4.13
> diff -u -p -r1.25.4.13 fccharset.c
> --- src/fccharset.c     7 Apr 2006 17:27:39 -0000       1.25.4.13
> +++ src/fccharset.c     11 Apr 2006 14:18:55 -0000
> @@ -168,6 +168,7 @@ FcCharSetPutLeaf (FcCharSet *fcs,
>          return FcFalse;
>       if (fcs->bank != FC_BANK_DYNAMIC)
>       {
> +        /* convert to dynamic */
>          int i;
> 
>          leaves = malloc ((fcs->num + 1) * sizeof (FcCharLeaf *));
> @@ -183,6 +184,10 @@ FcCharSetPutLeaf (FcCharSet        *fcs,
>              leaves[i] = FcCharSetGetLeaf(fcs, i);
>          memcpy (numbers, FcCharSetGetNumbers(fcs),
>                  fcs->num * sizeof (FcChar16));
> +
> +       fcs->bank = FC_BANK_DYNAMIC;
> +       fcs->u.dyn.leaves = leaves;
> +       fcs->u.dyn.numbers = numbers;
>       }
>       else
>       {
> 
> I''ve committed all of your patches as well as this one.
Hmm, the following two patches were not merged are still needed to fix
two memory leaks (unless I''m mistaken).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fccharsetmemleak.patch
Type: text/x-patch
Size: 1182 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/0273c174/fontconfig-2.3.94-fccharsetmemleak.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fclangmemleak2.patch
Type: text/x-patch
Size: 981 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/0273c174/fontconfig-2.3.94-fclangmemleak2.bin

The following patch fixes a memory leak in FcConfigBuildFonts (Coverity
defect #985) in error case.

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fccfgmemleak.patch
Type: text/x-patch
Size: 1263 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/116fbe52/fontconfig-2.3.94-fccfgmemleak.bin

The following patch fixes two memory leaks and one invalid memory access
(memory is access after being freed) in fcxml.c (Coverity defects #789,
#780, #781).
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcxmlfixes.patch
Type: text/x-patch
Size: 1536 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/5d401a54/fontconfig-2.3.94-fcxmlfixes.bin

This is a one-liner but I''m sure some compilers might warn about it ;)
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-semicolonfix.patch
Type: text/x-patch
Size: 850 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/c4efc122/fontconfig-2.3.94-semicolonfix-0001.bin

Le mardi 11 avril 2006 ? 17:39 +0200, Frederic Crozat a ?crit
:
> The following patch fixes a memory leak in FcConfigBuildFonts (Coverity
> defect #985) in error case.
Better patch attached, it fixes another memory leak in FcConfigAddBlank
too (Coverity defect #776).

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fccfgmemleak2.patch
Type: text/x-patch
Size: 1742 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060411/d6a9af46/fontconfig-2.3.94-fccfgmemleak2.bin

Frederic Crozat wrote:
> This is a one-liner but I''m sure some compilers might warn about
it ;)
All committed!  Thanks!

pat

Ok, I''m almost done with the various Coverity defects.

The only ones left are not trivial and requires much more knowledge of
fontconfig internal than I have, so I''ll explain the defects spot and
let people with more knowledge tell me if it is a real bug or if I
should close defect as false alarm :

-defect #984 in fcdir.c / FcDirScanConfig :
FcGlobalCacheReadDir might be call with config == NULL, which will call
FcConfigInodeMatchFontDir which deferences config without checking for
NULL value. I''m not sure how to fix this.

-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.

-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.

Oh and I think I found a coverity bug ;) (defect #782)..

There are two other memleaks in doc/edit-sgml.c but I don''t know if it
is worth trying to fix (defects #744, #745, leaking ss and ls in
DoReplace).

But now, we are done ;)
-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva

Frederic Crozat wrote:
> -defect #984 in fcdir.c / FcDirScanConfig :
> FcGlobalCacheReadDir might be call with config == NULL, which will call
> FcConfigInodeMatchFontDir which deferences config without checking for
> NULL value. I''m not sure how to fix this.
I''ve fixed this by not normalizing the dir name if we have a NULL 
config.  This means that it won''t necessarily find a cache that it 
should, but that''s just unfortunate, not critical.
> -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].
> -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?
> There are two other memleaks in doc/edit-sgml.c but I don''t know
if it
> is worth trying to fix (defects #744, #745, leaking ss and ls in
> DoReplace).
Not worth it.  edit-sgml is only run in the build process.
> But now, we are done ;)
Good, good.

pat

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.
> > -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

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

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


Also, Coverity updated their databases with yesterday fixes.

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.

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

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcfreetypefixes2.patch
Type: text/x-patch
Size: 1259 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060412/52421c5e/fontconfig-2.3.94-fcfreetypefixes2.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fontconfig-2.3.94-fcpatmemleak.patch
Type: text/x-patch
Size: 1519 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060412/52421c5e/fontconfig-2.3.94-fcpatmemleak.bin

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