Problems with a malloc'd char* to String

[Axel]

Hiho,

here my Newbie question (Win32 GUI):

I'm reading a file in binary mode to a char* named buffer. I used
malloc(filesize ) to make the needed space avaiable. The filedata in the
buffer seems to be ok..I can write the variable buffer back to a file
and the contents is ok. So I think until here all goes fine...

But now I want write the buffer contents to a Memo-Box for displaying.
It works...but only the first 4 or 5 letters are shown. I have no idea
how to get the whole buffer-variable into the String of the MemoBox
component for displaying.
Here is the part of my code:

..
..
..
int fsize = FileSizeByName( file); //Filesize
char *buffer = (char*)malloc(f size); //allocate buffer for filedata
fseek(filereade r, SEEK_SET, 0);
fread(buffer,fs ize,1,filereade r); //read binary data in buffer
Memo1->Text = buffer; //Memo1 displays buffer as String
free(buffer);
..
..
..

I hope I illustrated the problem right and somebody has an idea whats wrong.

Thanks in advance!!

Axel

[Peter van Merkerk]

Re: Problems with a malloc'd char* to String

> Hiho,[color=blue]
>
> here my Newbie question (Win32 GUI):
>
> I'm reading a file in binary mode to a char* named buffer. I used
> malloc(filesize ) to make the needed space avaiable. The filedata in[/color]
the[color=blue]
> buffer seems to be ok..I can write the variable buffer back to a file
> and the contents is ok. So I think until here all goes fine...
>
> But now I want write the buffer contents to a Memo-Box for displaying.
> It works...but only the first 4 or 5 letters are shown. I have no idea
> how to get the whole buffer-variable into the String of the MemoBox
> component for displaying.
> Here is the part of my code:
> .
> int fsize = FileSizeByName( file); file://Filesize
> char *buffer = (char*)malloc(f size); file://allocate buffer for[/color]
filedata[color=blue]
> fseek(filereade r, SEEK_SET, 0);
> fread(buffer,fs ize,1,filereade r); file://read binary data in buffer
> Memo1->Text = buffer; file://Memo1 displays buffer as String
> free(buffer);
> .
> I hope I illustrated the problem right and somebody has an idea whats[/color]
wrong.

After seeing Memo1->Text I think you are using Borland C++ Builder with
VCL controls. The Memo control and the FileSizeByName( ) function are not
part of the C++ standard. Questions specific to the Borland compiler and
VCL library should go to one of the newsgroups on the
newsgroups.borl and.com newsserver, as this group only discusses the
standard C++ language. See also: http://www.slack.net/~shiva/welcome.txt
and http://home.wanadoo.nl/efx/c++-faq/.

Though it is difficult to tell what goes wrong without knowing what
exactly is being read from the file I suspect your problem is that the
string read from the file is not zero terminated. The (untested) code
below should fix that:

int fsize = FileSizeByName( file);
char *buffer = new char[fsize+1];
fseek(filereade r, SEEK_SET, 0);
fread(buffer,fs ize,1,filereade r);
buffer[fsize] = 0; // Zero terminate string.
Memo1->Text = buffer;
delete []buffer;

HTH
--
Peter van Merkerk
peter.van.merke rk(at)dse.nl

[Axel]

Re: Problems with a malloc'd char* to String

Peter van Merkerk wrote:
[color=blue]
>
> After seeing Memo1->Text I think you are using Borland C++ Builder with
> VCL controls. The Memo control and the FileSizeByName( ) function are not
> part of the C++ standard. Questions specific to the Borland compiler and
> VCL library should go to one of the newsgroups on the
> newsgroups.borl and.com newsserver, as this group only discusses the
> standard C++ language. See also: http://www.slack.net/~shiva/welcome.txt
> and http://home.wanadoo.nl/efx/c++-faq/.
>
> Though it is difficult to tell what goes wrong without knowing what
> exactly is being read from the file I suspect your problem is that the
> string read from the file is not zero terminated. The (untested) code
> below should fix that:
>
> int fsize = FileSizeByName( file);
> char *buffer = new char[fsize+1];
> fseek(filereade r, SEEK_SET, 0);
> fread(buffer,fs ize,1,filereade r);
> buffer[fsize] = 0; // Zero terminate string.
> Memo1->Text = buffer;
> delete []buffer;
>
> HTH
> --
> Peter van Merkerk
> peter.van.merke rk(at)dse.nl
>[/color]
Hi,

thanks for your ideas. Please forget the Memo1->Text..it could be a
simple String-type variable too. Memo1 is only used for testing what
buffer contains. Later it will be something like:

String str = buffer;

And very later the end-goal is to code str with base64. But thats not
the problem at the moment.

But str only contains the first view chars of char*. Your solution
brings the same result. :-(

Axel

[Peter van Merkerk]

Re: Problems with a malloc'd char* to String

> Hi,[color=blue]
>
> thanks for your ideas. Please forget the Memo1->Text..it could be a
> simple String-type variable too. Memo1 is only used for testing what
> buffer contains. Later it will be something like:
>
> String str = buffer;
>
> And very later the end-goal is to code str with base64. But thats not
> the problem at the moment.
>
> But str only contains the first view chars of char*. Your solution
> brings the same result. :-([/color]

I cannot see obvious errors in the (incomplete) code you posted. Are you
sure the file doesn't have 0 bytes in it? Did you open the file in
binary or text mode? It would help if you post minimal but complete
(==compilable) code that exhibits the problem.

--
Peter van Merkerk
peter.van.merke rk(at)dse.nl

[Axel]

Re: Problems with a malloc'd char* to String


Peter van Merkerk wrote:
[color=blue]
>
> I cannot see obvious errors in the (incomplete) code you posted. Are you
> sure the file doesn't have 0 bytes in it? Did you open the file in
> binary or text mode? It would help if you post minimal but complete
> (==compilable) code that exhibits the problem.
>
> --
> Peter van Merkerk
> peter.van.merke rk(at)dse.nl
>
>[/color]

Ok..here is the complete codepart. The file is ok. Like written before I
can write it back with fwrite from the buffer-variable.

....
FILE *filereader = fopen("c:\\test \\original.doc" ,"rb");
if(!filereader)
{
//Errormessage: File not found
}
else
{
int fsize = FileSizeByName( file);
char *buffer = (char*)malloc(f size);
fseek(filereade r, SEEK_SET, 0);
fread(buffer,fs ize,1,filereade r);

String s = (String)buffer;

//for testing only lets write the file back
//and check if it is equal to the origin
FILE *filewriter = fopen("c:\\test \\copy.doc","wb ");
fwrite(buffer,f size,1,filewrit er);
fclose(filewrit er);
free(buffer);
}
fclose(fileread er);
....

String s contains not the whole buffer. Have I to allocate memory for
it? I have no idea.

Axel

[Default User]

Re: Problems with a malloc'd char* to String

Axel wrote:[color=blue]
>
> Peter van Merkerk wrote:[/color]
[color=blue]
> FILE *filereader = fopen("c:\\test \\original.doc" ,"rb");[/color]

You can write that filename as "c:/test/original.doc".

[color=blue]
> int fsize = FileSizeByName( file);[/color]

Nonstandard function. However, if you have the filename in the variable
file, why not use that above in the fopen() call?
[color=blue]
> char *buffer = (char*)malloc(f size);[/color]

In C++, new is preferred to malloc(), although for a basic type it
doesn't matter much.
[color=blue]
> fseek(filereade r, SEEK_SET, 0);[/color]

rewind() is shorter.
[color=blue]
> fread(buffer,fs ize,1,filereade r);
>
> String s = (String)buffer;[/color]

String is nonstandard. It would be a good idea to use std::string
instead.
[color=blue]
> String s contains not the whole buffer.[/color]

As you were reading in binary mode, not text mode, it is likely that
there was a byte equal to 0, which will truncate the string. Why are you
opening in binary? If it doesn't contain text, why are you trying to put
it in a string?




Brian Rodenborn

[Kevin Goodsell]

Re: Problems with a malloc'd char* to String

Axel wrote:
[color=blue]
>
> Ok..here is the complete codepart. The file is ok. Like written before I
> can write it back with fwrite from the buffer-variable.[/color]

This is not a complete program. I cannot copy/paste it into my editor,
save & compile it.
[color=blue]
>
> ...
> FILE *filereader = fopen("c:\\test \\original.doc" ,"rb");[/color]

When there's an input file for the program, it helps if you supply it as
well.
[color=blue]
> if(!filereader)
> {
> //Errormessage: File not found
> }
> else
> {
> int fsize = FileSizeByName( file);
> char *buffer = (char*)malloc(f size);
> fseek(filereade r, SEEK_SET, 0); fread(buffer,fs ize,1,filereade r);
> String s = (String)buffer;[/color]

"String" is not a standard C++ type, so we can't really help you with
that. I don't know the interface for it, so I don't know how one would
convert a C-style string to a String.

That cast looks highly suspicious, though. I strongly recommend getting
rid of it. If this causes errors, then it's almost certain that the cast
is only shutting up the compiler and forcing it to generate incorrect code.

If you were using std::string instead of this "String" thing, you could
do something like this:

std::string s(buffer, fsize);

I have no idea what the equivalent thing for "String" would be.
[color=blue]
>
> //for testing only lets write the file back
> //and check if it is equal to the origin
> FILE *filewriter = fopen("c:\\test \\copy.doc","wb ");
> fwrite(buffer,f size,1,filewrit er);
> fclose(filewrit er);
> free(buffer);
> }
> fclose(fileread er);
> ...
>
> String s contains not the whole buffer. Have I to allocate memory for
> it? I have no idea.[/color]

I'd say it's almost certain that you are using String incorrectly. But
if you want help with it, you'll have to ask on a group that supports
whatever library it comes from.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Kevin Goodsell]

Re: Problems with a malloc'd char* to String

Default User wrote:
[color=blue]
>
> In C++, new is preferred to malloc(), although for a basic type it
> doesn't matter much.
>[/color]

That's true, assuming that you use malloc() correctly. It just so
happens that people use it incorrectly quite often. 'new' avoids many of
the problems people have with malloc().

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Default User]

Re: Problems with a malloc'd char* to String

Kevin Goodsell wrote:[color=blue]
>
> Default User wrote:
>[color=green]
> >
> > In C++, new is preferred to malloc(), although for a basic type it
> > doesn't matter much.
> >[/color]
>
> That's true, assuming that you use malloc() correctly. It just so
> happens that people use it incorrectly quite often. 'new' avoids many of
> the problems people have with malloc().[/color]

Mainly due to typechecking, or lack thereof.

The biggest problem really comes with mixing the two forms, because you
can end up deleting memory allocated with malloc() or freeing memory
allocated via new. Bad things.




Brian Rodenborn

[Kevin Goodsell]

Re: Problems with a malloc'd char* to String

Default User wrote:
[color=blue]
> Kevin Goodsell wrote:
>[color=green]
>>Default User wrote:
>>
>>[color=darkred]
>>>In C++, new is preferred to malloc(), although for a basic type it
>>>doesn't matter much.
>>>[/color]
>>
>>That's true, assuming that you use malloc() correctly. It just so
>>happens that people use it incorrectly quite often. 'new' avoids many of
>>the problems people have with malloc().[/color]
>
>
> Mainly due to typechecking, or lack thereof.[/color]

The size calculation, also. People get that wrong quite often.
[color=blue]
>
> The biggest problem really comes with mixing the two forms, because you
> can end up deleting memory allocated with malloc() or freeing memory
> allocated via new. Bad things.[/color]

I haven't really seen this problem arise a lot, but it certainly would
not come as a surprise in code using both.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[llewelly]

Re: Problems with a malloc'd char* to String

Default User <first.last@com pany.com> writes:
[color=blue]
> Kevin Goodsell wrote:[color=green]
>>
>> Default User wrote:
>>[color=darkred]
>> >
>> > In C++, new is preferred to malloc(), although for a basic type it
>> > doesn't matter much.
>> >[/color]
>>
>> That's true, assuming that you use malloc() correctly. It just so
>> happens that people use it incorrectly quite often. 'new' avoids many of
>> the problems people have with malloc().[/color]
>
> Mainly due to typechecking, or lack thereof.
>
> The biggest problem really comes with mixing the two forms, because you
> can end up deleting memory allocated with malloc() or freeing memory
> allocated via new. Bad things.[/color]

In my experience, this is only common in projects that started out C
and were 'converted' to C++ .

[Default User]

Re: Problems with a malloc'd char* to String

Kevin Goodsell wrote:[color=blue]
>
> Default User wrote:[/color]
[color=blue][color=green][color=darkred]
> >>That's true, assuming that you use malloc() correctly. It just so
> >>happens that people use it incorrectly quite often. 'new' avoids many of
> >>the problems people have with malloc().[/color]
> >
> >
> > Mainly due to typechecking, or lack thereof.[/color]
>
> The size calculation, also. People get that wrong quite often.[/color]

If you use the handy-dandy comp.lang.c method, it's harder to screw up:

char *str;

str = (char*) malloc (num * sizeof *str);

[color=blue][color=green]
> > The biggest problem really comes with mixing the two forms, because you
> > can end up deleting memory allocated with malloc() or freeing memory
> > allocated via new. Bad things.[/color]
>
> I haven't really seen this problem arise a lot, but it certainly would
> not come as a surprise in code using both.[/color]


I've seen it. It's one of those things that'll probably work out ok, but
it's UB.



Brian Rodenborn

[Default User]

Re: Problems with a malloc'd char* to String

llewelly wrote:[color=blue]
>
> Default User <first.last@com pany.com> writes:[/color]
[color=blue][color=green]
> > The biggest problem really comes with mixing the two forms, because you
> > can end up deleting memory allocated with malloc() or freeing memory
> > allocated via new. Bad things.[/color]
>
> In my experience, this is only common in projects that started out C
> and were 'converted' to C++ .[/color]


Which are the ones likely to have malloc() and friends.




Brian Rodenborn

[Noah Roberts]

Re: Problems with a malloc'd char* to String

[color=blue][color=green][color=darkred]
>>>>Mainly due to typechecking, or lack thereof.
>>>
>>>The size calculation, also. People get that wrong quite often.[/color]
>>
>>If you use the handy-dandy comp.lang.c method, it's harder to screw up:
>>
>>char *str;
>>
>>str = (char*) malloc (num * sizeof *str);[/color]
>
>
> Funny you should demo it with the one type with which it is usually a
> mistake. Most people point a char* at a malloc'd block to have
> dynamicly sized string. That block, if used for a string, would be
> a very short string. ('\0' terminator only.)[/color]

Actually that depends on what num is. First off, I would like to say
that I object to the use of *str as the argument of sizeof. It is much
more confusing to say that rather than "sizeof char" (or in my taste:
"sizeof(cha r)") which is really what you want. It is also too easy to
make the mistake of leaving the '*' out and then you have a different
size all together. A third reason I don't like using *str is that it
looks to much like you are multiplying str by num and 'sizeof'. Of
course anyone who knows C knows that isn't the case but even then that
is what the mind wants to do with it. In reality the above code is
really bad in the readability area; it is totally correct and makes
sence but doesn't look right and doesn't taste well in my brain.

I would preffer:

char *str = NULL; // or 0 if you insist

str = (char*)malloc(( num + 1) * sizeof(char)); // +1 because num is
probably the length of my array.

[Kevin Goodsell]

Re: Problems with a malloc'd char* to String

Noah Roberts wrote:[color=blue]
>
> Actually that depends on what num is. First off, I would like to say
> that I object to the use of *str as the argument of sizeof. It is much
> more confusing to say that rather than "sizeof char"[/color]

That would be illegal. And in my opinion, it's not any more confusing.
Idioms are only confusing the first few times.
[color=blue]
> (or in my taste:
> "sizeof(cha r)") which is really what you want.[/color]

Not me. I want n objects of the type str points to, whatever that may be.

I don't like having (artificial) dependencies between the variable and
it's type, nor do I like language features which require that. They tend
to cause errors. Examples: printf/scanf require explicit knowledge of
the type involved, and are frequently used incorrectly. <limits.h>
macros for min and max value also require explicit knowledge of the type
involved. As far as I can tell, hungarian notation was created primarily
to deal with this type of problem, but it takes the wrong approach by
tagging the extra information on rather than eliminating the dependency.
Note that C++ replaces both of the examples with things that
automatically work for whatever type you feed them (overloads for I/O,
std::numeric_li mits requires the type, but at least it allows the type
to be aliased with a typedef). This is nice, because types can change
without much concern, maintenance is simplified, and errors are reduced
because there are simply fewer opportunities for them.

Now, the comp.lang.c-approved method for mallocing is not quite what
Brian posted, it's actually this:

// given:
char *str;

str = malloc(n * sizeof *str);

Without the cast, there is nothing that needs to be updated if str
changes to unsigned char *, or wchar_t *, or whatever.

Of course, this is not valid in C++. The cast is required. So you can't
escape having type information in the malloc call statement. That's part
of what makes 'new' better - no cast. And implicit type-checking. But it
does still require type information, which has always bugged me a
little. I'd like to use:

p = new typeof(*p)[n];

Well, maybe not, because it's kind of ugly. Instead, I could do this:

typedef int mytype;

mytype *p;
// ...
p = new mytype[n];

Which handles changing the type - you just update the typedef instead.
This generally isn't so bad - you can give the type a name that
describes its use, and drop in a replacement any time (maybe you need a
wider integer type, or you create a class that's more suited to the task).

I'm kind of straying from my main point, though. Basically, it's this: I
prefer to limit the number of related occurrences of a type name in a
module as much as possible, if there's a chance the type could be
updated later. More generally, the goal is to reduce points of
dependency within the source code, making simple maintenance a breeze.
Applying that idea to malloc, the comp.lang.c-approved method prevents
introducing points of dependency. 'new' makes it difficult, but aliasing
the type behind a typedef can fix it. The same fix would technically
work for malloc:

p = (mytype *)malloc(n * sizeof(mytype)) ;

I don't see any maintenance problems or excessive points of dependency.
But it does tie the statement to the typedefed type, if not the
underlying type, so it's not quite as good as the comp.lang.c-approved
method, which ties the statement to absolutely no particular type at all.
[color=blue]
> It is also too easy to
> make the mistake of leaving the '*' out and then you have a different
> size all together.[/color]

It's easy to make a lot of typos. It's an idiom for a reason - people
should get used to using it, used to seeing it, and immediately spot a
problem like that if it occurs. It's not an invalid concern, but it's a
relatively minor one.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.