How to free the variable
-
It looks like you're getting confused by what a pointer is. A pointer is just a variable that can be set to the addresses of arbitrary chunks of system memory. You don't need to "free"
readerbut you have to release whatever it points to back to the compiler's runtime. In the case you've presented all you have to do is:delete [] reader;
when you've finished with the block of memory the pointer points to. However the way you've written the code is a bit crap - if anything throws an exception between calling
bReadFileand thedeleteyou're going to leak memory. Instead of using an array consider using something with a bit more behavioural intelligence - e.g. std::vector. Then you'll not have to worry about cleaning up after yourself:std::vector read_from_file( std::size_t bytes_to_read )
{
std::vector bytes_read( bytes_to_read );
std::size_t number_bytes_read = 0;
ReadFile( handle_of_file, &bytes_read[ 0 ], &number_of_bytes_read, 0 );
return bytes_read;
}The code there will be within 5% of the performance of what you've written (faster on some compilers as there's no pointer aliasing) AND exception safe. Cheers, Ash PS: Anyone who thinks there's an expensive copy of the vector returned to the caller should upgrade their compiler.
Since you're bringing performance up, the output parameter alternative might be a lot faster under some conditions. Especially when you're calling read_from_file() several times in a loop, which is most likely.
std::vector<char>& read_from_file( std::size_t bytes_to_read, std::vector<char> &bytes_read )
{
bytes_read.reserve( bytes_to_read );
std::size_t number_bytes_read = 0;
ReadFile( handle_of_file, &bytes_read[ 0 ], &number_of_bytes_read, 0 );
bytes_read.resize(number_of_bytes_read);
return bytes_read;
}Edit: Forgot to resize the vector.
modified on Saturday, February 12, 2011 12:57 PM
-
Since you're bringing performance up, the output parameter alternative might be a lot faster under some conditions. Especially when you're calling read_from_file() several times in a loop, which is most likely.
std::vector<char>& read_from_file( std::size_t bytes_to_read, std::vector<char> &bytes_read )
{
bytes_read.reserve( bytes_to_read );
std::size_t number_bytes_read = 0;
ReadFile( handle_of_file, &bytes_read[ 0 ], &number_of_bytes_read, 0 );
bytes_read.resize(number_of_bytes_read);
return bytes_read;
}Edit: Forgot to resize the vector.
modified on Saturday, February 12, 2011 12:57 PM
3rd parameter is missing in the ReadFile() :thumbsup:nice info about output parameter. here is returned from the function. Can i use BYTE instead of char to read the file, is it ok or not.
Niklas Lindquist wrote:
bytes_read.reserve( bytes_to_read );
MODIFIED : <pre>std::vector<std::string> bytes_read;</pre> I guess bytes_read is a string type. I was bit confused. Now its ok.
Some Day I Will Prove MySelf :: GOLD
modified on Sunday, February 13, 2011 2:14 AM
-
3rd parameter is missing in the ReadFile() :thumbsup:nice info about output parameter. here is returned from the function. Can i use BYTE instead of char to read the file, is it ok or not.
Niklas Lindquist wrote:
bytes_read.reserve( bytes_to_read );
MODIFIED : <pre>std::vector<std::string> bytes_read;</pre> I guess bytes_read is a string type. I was bit confused. Now its ok.
Some Day I Will Prove MySelf :: GOLD
modified on Sunday, February 13, 2011 2:14 AM
goldenrose9 wrote:
3rd parameter is missing in the ReadFile()
You're quite right, and there's a misspelled one as well.
goldenrose9 wrote:
Can i use BYTE instead of char to read the file
std::vector<BYTE> bytes_read;
will do just fine. It really depends on how you would like to use the result. However
std::vectorstd::string bytes_read;
gives me the chills here. What exactly did you mean?
-
Since you're bringing performance up, the output parameter alternative might be a lot faster under some conditions. Especially when you're calling read_from_file() several times in a loop, which is most likely.
std::vector<char>& read_from_file( std::size_t bytes_to_read, std::vector<char> &bytes_read )
{
bytes_read.reserve( bytes_to_read );
std::size_t number_bytes_read = 0;
ReadFile( handle_of_file, &bytes_read[ 0 ], &number_of_bytes_read, 0 );
bytes_read.resize(number_of_bytes_read);
return bytes_read;
}Edit: Forgot to resize the vector.
modified on Saturday, February 12, 2011 12:57 PM
I'd have said that was true 10 years ago, but these days compilers have made that sort of trick fairly pointless. When compilers see a function of the form:
A some_function()
{
A a;/\* Other stuff \*/ return a;}
they're allowed (by the standard) to rewrite it as:
void some_function( memory_block_the_size_of_a &a )
{
new( &a ) A;/\* Other stuff \*/}
The compiler then converts the calls to that function from:
A a = some_function();
to something like:
memory_block_the_size_of_a block;
A &a( *reinterpret_cast<A *>( block ) );
some_function( &block );/* Other stuff */
a->~A();
which removes the copy construction which would normally happen with this sort of construct. The transformation the compiler does is a bit hard to represent in C++ as what it produces is usually exception safe while the representation I've done above isn't. That's the beauty of being a compiler I suppose! This transformation is called NRVO (named return value optimisation). It's fairly unique in that it's one of the few transformations the compiler may or may not do to some code which change it's visible behaviour. NRVO was implemented in VC++ 2005 and at least gcc 4.0, but it might have been a version or two earlier, can't remember without checking. Anyway, while you can use a reference parameter it (in my opinion at least) makes your code a lot more stilted as you end up writing two lines where you only needed one which was a direct statement of what you were trying to achieve. If you use the double ended reference type of function then you can actually end up with slower code if someone writes (naively):
A a = some_function( b );
as you still end up triggering the copy constructor. So the moral here is either provide an in/out parameter or return by value - don't fall halfway between. Lest anyone gets a bit slap happy and starts changing large swathes of code to this style it's worth noting that the compiler can't apply NRVO if: - The thing being returned isn't named (that's the named bit...) - There are multiple exits from the function, even if they're all returning the same thing - It's assignment not copy construction (so
A a = some_function()can invoke NRVO whilea = some_function()won't). In addition vendors don't have to implement it but most do - if they didn't they'd just make their compilers seem bad. While I'm banging on I should mentio -
i had tried both
delete reader
and
delete []reader
Some Day I Will Prove MySelf :: GOLD
In that case it sounds like you're mangling the heap somehow - some other pointer related operation is destroying a heap data structure so it can't work out what to do with the pointer you're giving it. The best thing to do in this case (which isn't practical in every case) is to go on a pointer purge and convert them to slightly less dangerous objects. In the short term check that the memory around your buffer isn't scribbled on by something else during it's lifetime. Cheers, Ash
-
Since you're bringing performance up, the output parameter alternative might be a lot faster under some conditions. Especially when you're calling read_from_file() several times in a loop, which is most likely.
std::vector<char>& read_from_file( std::size_t bytes_to_read, std::vector<char> &bytes_read )
{
bytes_read.reserve( bytes_to_read );
std::size_t number_bytes_read = 0;
ReadFile( handle_of_file, &bytes_read[ 0 ], &number_of_bytes_read, 0 );
bytes_read.resize(number_of_bytes_read);
return bytes_read;
}Edit: Forgot to resize the vector.
modified on Saturday, February 12, 2011 12:57 PM
-
I'd have said that was true 10 years ago, but these days compilers have made that sort of trick fairly pointless. When compilers see a function of the form:
A some_function()
{
A a;/\* Other stuff \*/ return a;}
they're allowed (by the standard) to rewrite it as:
void some_function( memory_block_the_size_of_a &a )
{
new( &a ) A;/\* Other stuff \*/}
The compiler then converts the calls to that function from:
A a = some_function();
to something like:
memory_block_the_size_of_a block;
A &a( *reinterpret_cast<A *>( block ) );
some_function( &block );/* Other stuff */
a->~A();
which removes the copy construction which would normally happen with this sort of construct. The transformation the compiler does is a bit hard to represent in C++ as what it produces is usually exception safe while the representation I've done above isn't. That's the beauty of being a compiler I suppose! This transformation is called NRVO (named return value optimisation). It's fairly unique in that it's one of the few transformations the compiler may or may not do to some code which change it's visible behaviour. NRVO was implemented in VC++ 2005 and at least gcc 4.0, but it might have been a version or two earlier, can't remember without checking. Anyway, while you can use a reference parameter it (in my opinion at least) makes your code a lot more stilted as you end up writing two lines where you only needed one which was a direct statement of what you were trying to achieve. If you use the double ended reference type of function then you can actually end up with slower code if someone writes (naively):
A a = some_function( b );
as you still end up triggering the copy constructor. So the moral here is either provide an in/out parameter or return by value - don't fall halfway between. Lest anyone gets a bit slap happy and starts changing large swathes of code to this style it's worth noting that the compiler can't apply NRVO if: - The thing being returned isn't named (that's the named bit...) - There are multiple exits from the function, even if they're all returning the same thing - It's assignment not copy construction (so
A a = some_function()can invoke NRVO whilea = some_function()won't). In addition vendors don't have to implement it but most do - if they didn't they'd just make their compilers seem bad. While I'm banging on I should mentioAs far as I know, NRVO would still have to issue a call the constructor of the return value every time the function is called. This constructor will then allocate heap memory for the vectors internal buffer. While NRVO will be reusing the same stack space for the vector object, it would not be able to re-use the memory for the internal buffer over subsequent calls to the function. The following usage of the function would only yield a single memory allocation, vector.reserve() in read_from_file(), no matter how many calls you make.
const size_t wanted_chunk_size = 512;
std::vector<char> buffer;
do
{
read_from_file(wanted_chunk_size, buffer);
write_to_somewhere(buffer);
}
while (buffer.size() == wanted_chunk_size);I might have to brush up on NRVO though. Please correct me if I'm wrong. I always find your posts interesting to read, so if you have time for an RVO post, that would be appreciated.
-
I'd have said that was true 10 years ago, but these days compilers have made that sort of trick fairly pointless. When compilers see a function of the form:
A some_function()
{
A a;/\* Other stuff \*/ return a;}
they're allowed (by the standard) to rewrite it as:
void some_function( memory_block_the_size_of_a &a )
{
new( &a ) A;/\* Other stuff \*/}
The compiler then converts the calls to that function from:
A a = some_function();
to something like:
memory_block_the_size_of_a block;
A &a( *reinterpret_cast<A *>( block ) );
some_function( &block );/* Other stuff */
a->~A();
which removes the copy construction which would normally happen with this sort of construct. The transformation the compiler does is a bit hard to represent in C++ as what it produces is usually exception safe while the representation I've done above isn't. That's the beauty of being a compiler I suppose! This transformation is called NRVO (named return value optimisation). It's fairly unique in that it's one of the few transformations the compiler may or may not do to some code which change it's visible behaviour. NRVO was implemented in VC++ 2005 and at least gcc 4.0, but it might have been a version or two earlier, can't remember without checking. Anyway, while you can use a reference parameter it (in my opinion at least) makes your code a lot more stilted as you end up writing two lines where you only needed one which was a direct statement of what you were trying to achieve. If you use the double ended reference type of function then you can actually end up with slower code if someone writes (naively):
A a = some_function( b );
as you still end up triggering the copy constructor. So the moral here is either provide an in/out parameter or return by value - don't fall halfway between. Lest anyone gets a bit slap happy and starts changing large swathes of code to this style it's worth noting that the compiler can't apply NRVO if: - The thing being returned isn't named (that's the named bit...) - There are multiple exits from the function, even if they're all returning the same thing - It's assignment not copy construction (so
A a = some_function()can invoke NRVO whilea = some_function()won't). In addition vendors don't have to implement it but most do - if they didn't they'd just make their compilers seem bad. While I'm banging on I should mentioAescleal wrote:
you can actually end up with slower code if someone writes (naively):
A a = some_function( b );
as you still end up triggering the copy constructor.
Aescleal wrote:
- It's assignment not copy construction (so A a = some_function() can invoke NRVO while a = some_function() won't).
Should I go sip another coffee or is there a contradiction? :confused: Otherwise: great post! :thumbsup: Didn't know about NRVO. You just gave me another weapon to fight multiple returns! :cool:
-
Aescleal wrote:
you can actually end up with slower code if someone writes (naively):
A a = some_function( b );
as you still end up triggering the copy constructor.
Aescleal wrote:
- It's assignment not copy construction (so A a = some_function() can invoke NRVO while a = some_function() won't).
Should I go sip another coffee or is there a contradiction? :confused: Otherwise: great post! :thumbsup: Didn't know about NRVO. You just gave me another weapon to fight multiple returns! :cool:
-
I think you can have that coffee. For NRVO to kick in, the returned value has to be created within that function.
Thanks, I'm better now. :) My issue was with the paragraph preceding my first quote - I didn't get what was meant by 'double ended reference type of function'.