Coverity Scan results

[davelab6]

Please find the latest report on new defect(s) introduced to FontForge found with Coverity Scan. Showing 20 of 1970 defect(s)

• This:

*** CID 1225112:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
/fontforgeexe/lookupui.c: 3945 in SFDTrimUndoOldToNew()
3939         char* ret = FileToAllocatedString( retf );
3940         fclose(retf);
3941         return ret;
3942
3943     error3SFDTrimUndoOldToNew: free(of);
3944     error2SFDTrimUndoOldToNew: free(nf);
>>>     CID 1225112:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
>>>     Calling "free(void *)" frees "retf" using "free" but it should have been freed using "fclose".
3945     error1SFDTrimUndoOldToNew: free(retf);
3946     error0SFDTrimUndoOldToNew:
3947         return 0;
3948     }
3949
3950

• This:

*** CID 1225111:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
/fontforgeexe/lookupui.c: 3943 in SFDTrimUndoOldToNew()
3937            goto error1SFDTrimUndoOldToNew;
3938
3939         char* ret = FileToAllocatedString( retf );
3940         fclose(retf);
3941         return ret;
3942
>>>     CID 1225111:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
>>>     Calling "free(void *)" frees "of" using "free" but it should have been freed using "fclose".
3943     error3SFDTrimUndoOldToNew: free(of);
3944     error2SFDTrimUndoOldToNew: free(nf);
3945     error1SFDTrimUndoOldToNew: free(retf);
3946     error0SFDTrimUndoOldToNew:
3947         return 0;
3948     }

• This:

*** CID 1225110:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
/fontforgeexe/lookupui.c: 3944 in SFDTrimUndoOldToNew()
3938
3939         char* ret = FileToAllocatedString( retf );
3940         fclose(retf);
3941         return ret;
3942
3943     error3SFDTrimUndoOldToNew: free(of);
>>>     CID 1225110:  Incorrect deallocator used  (ALLOC_FREE_MISMATCH)
>>>     Calling "free(void *)" frees "nf" using "free" but it should have been freed using "fclose".
3944     error2SFDTrimUndoOldToNew: free(nf);
3945     error1SFDTrimUndoOldToNew: free(retf);
3946     error0SFDTrimUndoOldToNew:
3947         return 0;
3948     }
3949

• This:

*** CID 1225116:  Unchecked return value from library  (CHECKED_RETURN)
/fontforge/ufo.c: 1255 in WriteUFOLayer()
1249
1250     int WriteUFOLayer(const char * glyphdir, SplineFont * sf, int layer) {
1251         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
1252         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
1253         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Make the dict.
1254
>>>     CID 1225116:  Unchecked return value from library  (CHECKED_RETURN)
>>>     No check of the return value of "GFileMkDir(glyphdir)".
1255         GFileMkDir( glyphdir );
1256         int i;
1257         SplineChar * sc;
1258         int err;
1259         for ( i=0; i<sf->glyphcnt; ++i ) if ( SCWorthOutputting(sc=sf->glyphs[i]) ) {
1260            PListAddString(dictnode,sc->name,sc->glif_name); // Add the glyph to the table of contents.

• This:

*** CID 1225115:  Unchecked return value from library  (CHECKED_RETURN)
/fontforge/parsettf.c: 698 in regionchecksum()
692         }
693     }
694
695     static uint32 regionchecksum(FILE *file, int start, int len) {
696         uint32 sum = 0, chunk;
697
>>>     CID 1225115:  Unchecked return value from library  (CHECKED_RETURN)
>>>     No check of the return value of "fseek(file, start, 0)".
698         fseek(file,start,SEEK_SET);
699         if ( len!=-1 ) len=(len+3)>>2;
700         while ( len==-1 || --len>=0 ) {
701             chunk = getlong(file);
702             if ( feof(file))
703         break;

• This:

*** CID 1225114:  Unchecked return value from library  (CHECKED_RETURN)
/fontforge/ufo.c: 1289 in WriteUFOFontFlex()
1283         if (asprintf(&foo, "rm -rf %s", basedir) >= 0) {
1284           if (system( foo ) == -1) fprintf(stderr, "Error clearing %s.\n", basedir);
1285           free( foo ); foo = NULL;
1286         }
1287
1288         /* Create it */
>>>     CID 1225114:  Unchecked return value from library  (CHECKED_RETURN)
>>>     No check of the return value of "GFileMkDir(basedir)".
1289         GFileMkDir( basedir );
1290
1291         err  = !UFOOutputMetaInfo(basedir,sf);
1292         err |= !UFOOutputFontInfo(basedir,sf,layer);
1293         err |= !UFOOutputGroups(basedir,sf);
1294         err |= !UFOOutputKerning(basedir,sf);

• This:

*** CID 1225113:  Ignoring number of bytes read  (CHECKED_RETURN)
/fontforge/palmfonts.c: 345 in SFReadPalmPdb()
339     return( NULL );
340
341         fseek(file,0,SEEK_END);
342         file_end = ftell(file);
343         fseek(file,0,SEEK_SET);
344
>>>     CID 1225113:  Ignoring number of bytes read  (CHECKED_RETURN)
>>>     "fread(void * restrict, size_t, size_t, FILE * restrict)" returns the number of bytes read, but it is ignored.
345         fread(name,1,32,file);
346         if ( ferror(file) )
347       goto fail;
348         name[32]=0;
349         fseek(file,0x2c,SEEK_CUR);          /* Find start of record list */
350         num_records = getushort(file);

• This:

*** CID 1225124:  Logically dead code  (DEADCODE)
/fontforge/featurefile.c: 2129 in fea_classesIntersect()
2123             }
2124             *pt1 = ch1; // Restore the byte.
2125         }
2126         break_point = index; // Divide the entries from the two sources by index.
2127         // Parse the second input.
2128         for ( pt2=class2 ; output == 0; ) {
>>>     CID 1225124:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "while (*pt2 == ' ')
  ++pt2;".
2129             while ( *pt2==' ' ) ++pt2;
2130             if ( *pt2=='\0' )
2131                 output = -1; // We cancel further action if one list is blank.
2132             for ( start2 = pt2; *pt2!=' ' && *pt2!='\0'; ++pt2 );
2133             ch1 = *pt2; *pt2 = '\0'; // Cache the byte and terminate.
2134             struct glif_name * tmp = NULL;
/fontforge/featurefile.c: 2143 in fea_classesIntersect()
2137             } else if (tmp->gid < break_point) {
2138               output = 1;
2139             }
2140             *pt2 = ch2; // Restore the byte.
2141         }
2142         glif_name_hash_destroy(glif_name_hash); // Close the hash table.
>>>     CID 1225124:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "return 1;".
2143         if (output == 1) return 1;
2144         return 0;
2145     #else
2146         for ( pt1=class1 ; ; ) {
2147             while ( *pt1==' ' ) ++pt1;
2148             if ( *pt1=='\0' )

• This:

*** CID 1225123:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 773 in UFOOutputFontInfo()
767         return true;
768     }
769
770     static int UFOOutputFontInfo(const char *basedir, SplineFont *sf, int layer) {
771         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
772         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225123:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
773         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Add the dict.
774
775         DBounds bb;
776         double test;
777
778     /* Same keys in both formats */

• This:

*** CID 1225122:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 949 in UFOOutputGroups()
943         /* These don't act like fontforge's groups. There are comments that this */
944         /*  could be used for defining classes (kerning classes, etc.) but no */
945         /*  resolution saying that the actually are. */
946         /* Should I omit a file I don't use? Or leave it blank? */
947         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
948         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225122:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
949         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Add the dict.
950         // TODO: Maybe output things.
951         char *fname = buildname(basedir, "groups.plist"); // Build the file name.
952         xmlSaveFormatFileEnc(fname, plistdoc, "UTF-8", 1); // Store the document.
953         free(fname); fname = NULL;
954         xmlFreeDoc(plistdoc); // Free the memory.

• This:

*** CID 1225121:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 974 in UFOOutputKerning()
968     static int UFOOutputKerning(const char *basedir, const SplineFont *sf) {
969         SplineChar *sc;
970         int i;
971
972         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
973         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225121:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
974         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Add the dict.
975
976         for ( i=0; i<sf->glyphcnt; ++i ) if ( SCWorthOutputting(sc=sf->glyphs[i]) && sc->kerns!=NULL )
977             KerningPListAddGlyph(dictnode,sc->name,sc->kerns);
978
979         char *fname = buildname(basedir, "kerning.plist"); // Build the file name.

• This:

*** CID 1225120:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 759 in UFOOutputMetaInfo()
753         }
754     }
755
756     static int UFOOutputMetaInfo(const char *basedir,SplineFont *sf) {
757         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
758         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225120:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
759         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Add the dict.
760         PListAddString(dictnode,"creator","net.GitHub.FontForge");
761         PListAddInteger(dictnode,"formatVersion",2);
762         char *fname = buildname(basedir, "metainfo.plist"); // Build the file name.
763         xmlSaveFormatFileEnc(fname, plistdoc, "UTF-8", 1); // Store the document.
764         free(fname); fname = NULL;

• This:

*** CID 1225119:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 993 in UFOOutputVKerning()
987     static int UFOOutputVKerning(const char *basedir, const SplineFont *sf) {
988         SplineChar *sc;
989         int i;
990
991         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
992         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225119:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
993         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Add the dict.
994
995         for ( i=sf->glyphcnt-1; i>=0; --i ) if ( SCWorthOutputting(sc=sf->glyphs[i]) && sc->vkerns!=NULL ) break;
996         if ( i<0 ) return( true );
997         for ( i=0; i<sf->glyphcnt; ++i ) if ( (sc=sf->glyphs[i])!=NULL && sc->vkerns!=NULL )
998             KerningPListAddGlyph(dictnode,sc->name,sc->vkerns);

• This:

*** CID 1225118:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 1253 in WriteUFOLayer()
1247             return name_numbered;
1248     }
1249
1250     int WriteUFOLayer(const char * glyphdir, SplineFont * sf, int layer) {
1251         xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
1252         xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225118:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
1253         xmlNodePtr dictnode = xmlNewChild(rootnode, NULL, BAD_CAST "dict", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Make the dict.
1254
1255         GFileMkDir( glyphdir );
1256         int i;
1257         SplineChar * sc;
1258         int err;

• This:

*** CID 1225117:  Logically dead code  (DEADCODE)
/fontforge/ufo.c: 1348 in WriteUFOFontFlex()
1342           memset(layer_path_hash, 0, sizeof(struct glif_name_index));
1343     #else
1344           void * layer_name_hash = NULL;
1345     #endif
1346           xmlDocPtr plistdoc = PlistInit(); if (plistdoc == NULL) return false; // Make the document.
1347           xmlNodePtr rootnode = xmlDocGetRootElement(plistdoc); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Find the root node.
>>>     CID 1225117:  Logically dead code  (DEADCODE)
>>>     Execution cannot reach this statement "xmlFreeDoc(plistdoc);".
1348           xmlNodePtr arraynode = xmlNewChild(rootnode, NULL, BAD_CAST "array", NULL); if (rootnode == NULL) { xmlFreeDoc(plistdoc); return false; } // Make the dict.
1349
1350           int layer_pos;
1351           for (layer_pos = 0; layer_pos < sf->layer_cnt; layer_pos++) {
1352             glyphdir = buildname(basedir,"glyphs");
1353             xmlNodePtr layernode = xmlNewChild(arraynode, NULL, BAD_CAST "array", NULL);

• This:

*** CID 1225129:  Dereference after null check  (FORWARD_NULL)
/fontforge/splineoverlap.c: 804 in AddSpline()
798             else if (Within4RoundingErrors(m->tstart, m->tend))
799                 SOError( "Attempt to subset monotonic rejoin inappropriately: m->tstart and m->tend are very close (%f = %f, t = %f)\n",
800                         m->tstart, m->tend, t );
801             else if (Within4RoundingErrors(m->tstart, m->tend))
802                 SOError( "Attempt to subset monotonic rejoin inappropriately: m->tstart and m->tend are very close (%f = %f, t = %f)\n",
803                         m->tstart, m->tend, t );
>>>     CID 1225129:  Dereference after null check  (FORWARD_NULL)
>>>     Dereferencing null pointer "m->s".
804             else if (Within4RoundingErrors(m->s->from->me.x,m->s->to->me.x) && Within4RoundingErrors(m->s->from->me.y,m->s->to->me.y))
805                 SOError( "The curve is too short.\n");
806             else {
807                 /* It is monotonic, so a subset of it must also be */
808                 Monotonic *m2 = chunkalloc(sizeof(Monotonic));
809                 BasePoint pt, inter;

• This:

*** CID 1225128:  Dereference after null check  (FORWARD_NULL)
/fontforge/splineoverlap.c: 2288 in MonotonicFindAt()
2282            } else
2283         break;
2284            /* If the next monotonic continues in the same direction, and we found*/
2285            /*  it too, then don't count both. They represent the same intersect */
2286            /* If they are in oposite directions then they cancel each other out */
2287            /*  and that is correct */
>>>     CID 1225128:  Dereference after null check  (FORWARD_NULL)
>>>     Dereferencing null pointer "mm".
2288            if ( mm!=m &&   /* Should always be true */
2289                    (&mm->xup)[which]==(&m->xup)[which] ) {
2290                for ( j=cnt-1; j>=0; --j )
2291                    if ( space[j]==mm )
2292                break;
2293                if ( j!=-1 ) {
/fontforge/splineoverlap.c: 2288 in MonotonicFindAt()
2282            } else
2283         break;
2284            /* If the next monotonic continues in the same direction, and we found*/
2285            /*  it too, then don't count both. They represent the same intersect */
2286            /* If they are in oposite directions then they cancel each other out */
2287            /*  and that is correct */
>>>     CID 1225128:  Dereference after null check  (FORWARD_NULL)
>>>     Dereferencing null pointer "mm".
2288            if ( mm!=m &&   /* Should always be true */
2289                    (&mm->xup)[which]==(&m->xup)[which] ) {
2290                for ( j=cnt-1; j>=0; --j )
2291                    if ( space[j]==mm )
2292                break;
2293                if ( j!=-1 ) {

• This:

*** CID 1225127:  Dereference after null check  (FORWARD_NULL)
/fontforge/ufo.c: 713 in PListAddPrivateArray()
707                 if (skipping)
708                     ++value;
709                 else
710                     tmp[tmppos++] = *value++;
711                 if (tmppos == tmpsize) { tmpsize *= 2; tmp = realloc(tmp, tmpsize); }
712             }
>>>     CID 1225127:  Dereference after null check  (FORWARD_NULL)
>>>     Dereferencing null pointer "tmp".
713             tmp[tmppos] = '\0';
714             if (tmp != NULL) {
715               xmlNewChildString(arrayxml, NULL, BAD_CAST "integer", BAD_CAST tmp); // "<integer>%s</integer>" tmp
716               free(tmp); tmp = NULL;
717             }
718             while ( *value==' ' ) ++value;

• This:

*** CID 1225126:  Explicit null dereferenced  (FORWARD_NULL)
/fontforge/ufo.c: 1834 in _UFOLoadGlyph()
1828
1829                            // If we have not identified the contour as holding an anchor point, we continue processing it as a rendered shape.
1830                            int wasquad = -1; // This tracks whether we identified the previous curve as quadratic. (-1 means undefined.)
1831                            int firstpointsaidquad = -1; // This tracks the declared order of the curve leading into the first on-curve point.
1832
1833                        ss = chunkalloc(sizeof(SplineSet));
>>>     CID 1225126:  Explicit null dereferenced  (FORWARD_NULL)
>>>     Assigning: "ss->first" = "NULL".
1834                            ss->first = NULL;
1835
1836                            for ( points = contour->children; points!=NULL; points=points->next ) {
1837                            char *xs, *ys, *type, *pname, *smooths;
1838                            double x,y;
1839                            int smooth = 0;

• This:

*** CID 1225125:  Dereference after null check  (FORWARD_NULL)
/fontforge/ufo.c: 1797 in _UFOLoadGlyph()
1791                            // We now look for anchor points.
1792                 char *sname;
1793
1794                 for ( points=contour->children; points!=NULL; points=points->next )
1795                     if ( xmlStrcmp(points->name,(const xmlChar *) "point")==0 )
1796                 break;
>>>     CID 1225125:  Dereference after null check  (FORWARD_NULL)
>>>     Dereferencing null pointer "points".
1797                 for ( npoints=points->next; npoints!=NULL; npoints=npoints->next )
1798                     if ( xmlStrcmp(npoints->name,(const xmlChar *) "point")==0 )
1799                 break;
1800                            // If the contour has a single point without another point after it, we assume it to be an anchor point.
1801                 if ( points!=NULL && npoints==NULL ) {
1802                     sname = (char *) xmlGetProp(points, (xmlChar *) "name");

[adrientetar]

cc #1499

[adrientetar]

We could also run the scan on each Travis build, there's an interface for that.

[adrientetar]

Just uploaded a new scan and we have 198 new defects but I think that's because analysis were not run to completion previously or something since it's all in old code.

[adrientetar]

Total defects count is around 1970.

[davelab6]

How severe are these?
On 14 Jul 2014 15:23, "Adrien Tétar" notifications@github.com wrote:

Total defects count is around 1970.

—
Reply to this email directly or view it on GitHub
#1500 (comment)
.

[adrientetar]

It depends, a big part of these are memory leaks which take up memory but do not make the program crash. I'm trying to fix double frees right now because these can be hard to debug sometimes. There isn't too much of them.
Much of these bugs are quite old, a little part of them gets triggered on failing conditions like when FontForge fails to create a temporary file – these ones shouldn't be too bothersome in the real world but need to be fixed still.

[adrientetar]

I am proposing to close this because we have a lot of these, [some should be closed, others are unnecessary] (⇐ generally speaking I mean) and they should be triaged properly from the dashboard whatsoever.

[davelab6]

The Coverity Scan dashboard? I think its ok to leave this open, someone else might appear and want to work on this :)

[tshinnic]

I think the Coverity scanning can be a very good thing, even when it can get defects wrong. And we do need a place to talk about specific items and/or strategies.

First two line items I looked at today, the first is a leak, and the second isn't but the false positive can be worked around as two different variables were being juggled which confused the scanner.

Then I peeked at a third line item:

CID 1082770 (# 1 of 1): Uninitialized scalar variable (UNINIT)
8. uninit_use: Using uninitialized value width.

Uninitialized variable... well blaaahh, ya'know?
Except look at native scripting routine _AddHint()
Can you tell me that line 6031 isn't a simple and frustrating copy-n-paste error, a line copied from just above there but only one of two parts changed?

(hmm, no tests test AddHHint or AddVHint - how to easily do a test? Blaahh!)

[adrientetar]

@tshinnic Can you submit a Pull Request for it?

[tshinnic]

Done. #1655 with bonus double-free fix!

[skef]

There's a database of these.