[ECOS] Mixed use of delete with malloc in fclose

[ECOS] Mixed use of delete with malloc in fclose

[Guenter Ebermann]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

Hi All,

During searching a bug in our firmware (which was not related to ecos)
I stumbled accross this minor inconsitency (which does not have an effect
in our setup because the operator delete in libsupc++ of our gcc uses
free internally):

In packages/language/c/libc/stdio/current/src/common/fopen.cxx
line 172 (in Revision 1.9 - HEAD) memory is alloced with malloc and object is
constructed with placement new:

    // Allocate it some memory and construct it.
    curr_stream = (Cyg_StdioStream *)malloc(sizeof(*curr_stream));
    if (curr_stream == NULL) {
        cyg_stdio_close( dev );
        Cyg_libc_stdio_files::unlock();
        errno = ENOMEM;
        return NULL;
    } // if

    curr_stream = new ((void *)curr_stream) Cyg_StdioStream( dev, open_mode,
                                                             append, binary,
                                                             bufmode, bufsize );

But fclose frees memory useing delete (if it not overwritten with empty stubs).
Please not that the (void *) cast at the new statement is also useless.
For a proposed patch please see the attachment.

    -Guenter

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört?
Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

[-- Attachment #2: fclose_delete.patch --]
[-- Type: application/octet-stream, Size: 1055 bytes --]

Index: language/c/libc/stdio/current/src/common/fclose.cxx
===================================================================
RCS file: /home/users/cvs/ESW/ThirdParty/eCos_NodeMPC5200/packages/language/c/libc/stdio/current/src/common/fclose.cxx,v
retrieving revision 1.1.1.1
diff -u -5 -p -r1.1.1.1 fclose.cxx
--- language/c/libc/stdio/current/src/common/fclose.cxx	23 May 2006 13:28:40 -0000	1.1.1.1
+++ language/c/libc/stdio/current/src/common/fclose.cxx	27 Jun 2008 08:40:48 -0000
@@ -107,19 +107,15 @@ fclose( FILE *stream ) __THROW
         Cyg_libc_stdio_files::unlock();
         errno = err;
         return EOF;
     }
     
-#ifdef CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS
     // Explicitly call destructor - this flushes the output too
     real_stream->~Cyg_StdioStream();
 
     // and free it
     free(real_stream);
-#else
-    delete real_stream;
-#endif // CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS
 
     // and mark the stream available for use
     Cyg_libc_stdio_files::set_file_stream(i, NULL);
         
     Cyg_libc_stdio_files::unlock();

[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Mixed use of delete with malloc in fclose

[Jonathan Larmour]

Guenter Ebermann wrote:
> Hi All,
> 
> During searching a bug in our firmware (which was not related to ecos)
> I stumbled accross this minor inconsitency (which does not have an effect
> in our setup because the operator delete in libsupc++ of our gcc uses
> free internally):
> 
> In packages/language/c/libc/stdio/current/src/common/fopen.cxx
> line 172 (in Revision 1.9 - HEAD) memory is alloced with malloc and object is
> constructed with placement new:
> 
>     // Allocate it some memory and construct it.
>     curr_stream = (Cyg_StdioStream *)malloc(sizeof(*curr_stream));
>     if (curr_stream == NULL) {
>         cyg_stdio_close( dev );
>         Cyg_libc_stdio_files::unlock();
>         errno = ENOMEM;
>         return NULL;
>     } // if
> 
>     curr_stream = new ((void *)curr_stream) Cyg_StdioStream( dev, open_mode,
>                                                              append, binary,
>                                                              bufmode, bufsize );
> 
> But fclose frees memory useing delete (if it not overwritten with empty stubs).
> Please not that the (void *) cast at the new statement is also useless.
> For a proposed patch please see the attachment.

I'm not sure I understand the issue. If CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS
is not defined, then the implementation of delete in libsupc++ should call
free, just like you say is relevant for your setup.

The fact the constructor was called by placement new should be irrelevant.

Although I guess you could say the call to delete is redundant - you could
always run the code in the CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS block safely.

So just so I'm sure I understand it, this patch is not fixing a problem,
it's just a bit of cleanup, right?

When you reply you can provide a ChangeLog entry, thanks :-).

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Mixed use of delete with malloc in fclose

[Guenter Ebermann]

Jonathan Larmour wrote:
> I'm not sure I understand the issue. If CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS
> is not defined, then the implementation of delete in libsupc++ should call
> free, just like you say is relevant for your setup.

The theoretical problem would be if operator new and delete would
not use the same memory management as malloc and free does.

> Although I guess you could say the call to delete is 
> redundant - you could always run the code in the 
> CYGFUN_INFRA_EMPTY_DELETE_FUNCTIONS block safely.

Yes, the code will be a bit simpler too.
 
> So just so I'm sure I understand it, this patch is not fixing 
> a problem, it's just a bit of cleanup, right?

Its a sort of rule when implementing C++: If new is used delete must be
used. Otherwise if malloc is used free must be used. Mixing these functions
is not allowed.

> When you reply you can provide a ChangeLog entry, thanks :-).

Replaced use of delete with free because memory was allocated using malloc.

    -Guenter


-- 
Psssst! Schon das coole Video vom GMX MultiMessenger gesehen?
Der Eine für Alle: http://www.gmx.net/de/go/messenger03

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss