Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #6215 > unrolled thread

Persisting a structure: Valgrind errors

Back to article view | Back to comp.lang.c++

Contents

  Persisting a structure:  Valgrind errors Ramon <ramif_47.invalid@yahoo.co.uk> - 2011-06-04 23:19 +0200
    Re: Persisting a structure:  Valgrind errors Ian Collins <ian-news@hotmail.com> - 2011-06-05 10:59 +1200
      Re: Persisting a structure: Valgrind errors Ebenezer <woodbrian77@gmail.com> - 2011-06-04 18:11 -0700
        Re: Persisting a structure: Valgrind errors Öö Tiib <ootiib@hot.ee> - 2011-06-04 18:56 -0700
    Re: Persisting a structure:  Valgrind errors Juha Nieminen <nospam@thanks.invalid> - 2011-06-05 05:57 +0000
      Re: Persisting a structure:  Valgrind errors "io_x" <a@b.c.invalid> - 2011-06-05 08:14 +0200
        Re: Persisting a structure:  Valgrind errors Ramon <ramif_47.invalid@yahoo.co.uk> - 2011-06-05 11:40 +0200
          Re: Persisting a structure:  Valgrind errors "io_x" <a@b.c.invalid> - 2011-06-05 12:17 +0200
            Re: Persisting a structure:  Valgrind errors Ramon <ramif_47.invalid@yahoo.co.uk> - 2011-06-05 18:15 +0200
              Re: Persisting a structure:  Valgrind errors Asger-P <junk@asger-p.dk> - 2011-06-05 18:44 +0200
              Re: Persisting a structure:  Valgrind errors Juha Nieminen <nospam@thanks.invalid> - 2011-06-07 06:20 +0000
                Re: Persisting a structure: Valgrind errors Joshua Maurice <joshuamaurice@gmail.com> - 2011-06-07 00:24 -0700
                  Re: Persisting a structure: Valgrind errors Juha Nieminen <nospam@thanks.invalid> - 2011-06-07 08:40 +0000
                    Re: Persisting a structure: Valgrind errors Miles Bader <miles@gnu.org> - 2011-06-08 14:43 +0900
                      Re: Persisting a structure: Valgrind errors Joshua Maurice <joshuamaurice@gmail.com> - 2011-06-07 23:37 -0700
                        Re: Persisting a structure: Valgrind errors Miles Bader <miles@gnu.org> - 2011-06-08 17:18 +0900
          Re: Persisting a structure:  Valgrind errors Nobody <nobody@nowhere.com> - 2011-06-05 18:42 +0100
      Re: Persisting a structure: Valgrind errors Ebenezer <woodbrian77@gmail.com> - 2011-06-05 13:22 -0700
    Re: Persisting a structure:  Valgrind errors Asger-P <junk@asger-p.dk> - 2011-06-05 12:04 +0200

#6215 — Persisting a structure: Valgrind errors

Hello,

Though I have a good amount of experience in ANSI C, Java, C#...etc, I 
am fairly new with C++.

What I am trying to do is to persist a struct to a binary file.  The 
structure is being saved correctly yet when I am trying to load it from 
disk to memory Valgrind is popping up some errors (Invalid write and 
Invalid free) from this line of code:

file.read((char*) i, sizeof(struct record));

Attached with this message is the code of my program.  Can anyone point 
out what am I doing wrong?  Mind you, the program runs great, just 
Valgrind is showing some warnings.

Thanks.

//
// code
//

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

#define NAME "test_file.bin"


struct record
{
	string	name;
	string	surname;
};


void save(struct record *i)
{
	fstream file;
	
	file.open(NAME, fstream::trunc | fstream::binary | fstream::out);
	
	
	if (!file.is_open())
		throw string("Unable to open file");
	
	file.write((char*)i, sizeof(struct record));
	
	file.close();
}




void load(struct record *i)
{
	fstream 		file;
	
	file.open(NAME, fstream::binary | fstream::in);
	
	
	if (!file.is_open())
		throw string("Unable to open file");
	
	
  	file.read((char*) i, sizeof(struct record));	// error
	
	file.close();
}




int main()
{
	struct record *r = new struct record;
	struct record *test = new struct record;
	
	r->name = "name";
	r->surname = "surname";
	
	save(r);
  	load(test);
  	
   	cout << test->name << endl;
   	cout << test->surname << endl;
	
	delete r;
	delete test;
	
	return 0;
}

[toc] | [next] | [standalone]

#6219

On 06/ 5/11 09:19 AM, Ramon wrote:
> Hello,
>
> Though I have a good amount of experience in ANSI C, Java, C#...etc, I
> am fairly new with C++.
>
> What I am trying to do is to persist a struct to a binary file.  The
> structure is being saved correctly yet when I am trying to load it from
> disk to memory Valgrind is popping up some errors (Invalid write and
> Invalid free) from this line of code:
>
> file.read((char*) i, sizeof(struct record));
>
> Attached with this message is the code of my program.  Can anyone point
> out what am I doing wrong?  Mind you, the program runs great, just
> Valgrind is showing some warnings.

I'm not surprised!

> #include<iostream>
> #include<fstream>
> #include<string>
>
> using namespace std;
>
> #define NAME "test_file.bin"

Yuck, use a constant!

> struct record
> {
> 	string	name;
> 	string	surname;
> };
>
>
> void save(struct record *i)

This and all subsequent uses of "struct" are superfluous.

> {
> 	fstream file;
> 	
> 	file.open(NAME, fstream::trunc | fstream::binary | fstream::out);
> 	
> 	
> 	if (!file.is_open())
> 		throw string("Unable to open file");
> 	
> 	file.write((char*)i, sizeof(struct record));

This will write what ever the static part of the strings are, but 
probably not the actual data (the two may be the same for small strings 
if the small value optimisations are used, but who knows?).

> 	file.close();
> }
>
>
>
>
> void load(struct record *i)
> {
> 	fstream 		file;
> 	
> 	file.open(NAME, fstream::binary | fstream::in);
> 	
> 	
> 	if (!file.is_open())
> 		throw string("Unable to open file");
> 	
> 	
>    	file.read((char*) i, sizeof(struct record));	// error

This will write some random garbage over the strings, causing all sorts 
of errors when they are deleted.

If you are going to serialise a struct, you have to serialise each 
member.  What you are doing could work for a POD struct, but it would be 
unsafe even for those.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#6220 — Re: Persisting a structure: Valgrind errors

On Jun 4, 5:59 pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 06/ 5/11 09:19 AM, Ramon wrote:
>
> > Hello,
>
> > Though I have a good amount of experience in ANSI C, Java, C#...etc, I
> > am fairly new with C++.
>
> > What I am trying to do is to persist a struct to a binary file.  The
> > structure is being saved correctly yet when I am trying to load it from
> > disk to memory Valgrind is popping up some errors (Invalid write and
> > Invalid free) from this line of code:
>
> > file.read((char*) i, sizeof(struct record));
>
> > Attached with this message is the code of my program.  Can anyone point
> > out what am I doing wrong?  Mind you, the program runs great, just
> > Valgrind is showing some warnings.
>
> I'm not surprised!
>
> > #include<iostream>
> > #include<fstream>
> > #include<string>
>
> > using namespace std;
>
> > #define NAME "test_file.bin"
>
> Yuck, use a constant!
>
> > struct record
> > {
> >    string  name;
> >    string  surname;
> > };
>
> > void save(struct record *i)
>
> This and all subsequent uses of "struct" are superfluous.
>
> > {
> >    fstream file;
>
> >    file.open(NAME, fstream::trunc | fstream::binary | fstream::out);
>
> >    if (!file.is_open())
> >            throw string("Unable to open file");
>
> >    file.write((char*)i, sizeof(struct record));
>
> This will write what ever the static part of the strings are, but
> probably not the actual data (the two may be the same for small strings
> if the small value optimisations are used, but who knows?).
>
>
>
>
>
>
>
>
>
> >    file.close();
> > }
>
> > void load(struct record *i)
> > {
> >    fstream                 file;
>
> >    file.open(NAME, fstream::binary | fstream::in);
>
> >    if (!file.is_open())
> >            throw string("Unable to open file");
>
> >            file.read((char*) i, sizeof(struct record));    // error
>
> This will write some random garbage over the strings, causing all sorts
> of errors when they are deleted.
>
> If you are going to serialise a struct, you have to serialise each
> member.  What you are doing could work for a POD struct, but it would be
> unsafe even for those.
>

Agreed.  More info here --
http://en.wikipedia.org/wiki/Serialization

[toc] | [prev] | [next] | [standalone]

#6223 — Re: Persisting a structure: Valgrind errors

On Jun 5, 4:11 am, Ebenezer <woodbria...@gmail.com> wrote:
> On Jun 4, 5:59 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
>
>
> > On 06/ 5/11 09:19 AM, Ramon wrote:
>
> > > Hello,
>
> > > Though I have a good amount of experience in ANSI C, Java, C#...etc, I
> > > am fairly new with C++.
>
> > > What I am trying to do is to persist a struct to a binary file.  The
> > > structure is being saved correctly yet when I am trying to load it from
> > > disk to memory Valgrind is popping up some errors (Invalid write and
> > > Invalid free) from this line of code:
>
> > > file.read((char*) i, sizeof(struct record));
>
> > > Attached with this message is the code of my program.  Can anyone point
> > > out what am I doing wrong?  Mind you, the program runs great, just
> > > Valgrind is showing some warnings.
>
> > I'm not surprised!
>
> > > #include<iostream>
> > > #include<fstream>
> > > #include<string>
>
> > > using namespace std;
>
> > > #define NAME "test_file.bin"
>
> > Yuck, use a constant!
>
> > > struct record
> > > {
> > >    string  name;
> > >    string  surname;
> > > };
>
> > > void save(struct record *i)
>
> > This and all subsequent uses of "struct" are superfluous.
>
> > > {
> > >    fstream file;
>
> > >    file.open(NAME, fstream::trunc | fstream::binary | fstream::out);
>
> > >    if (!file.is_open())
> > >            throw string("Unable to open file");
>
> > >    file.write((char*)i, sizeof(struct record));
>
> > This will write what ever the static part of the strings are, but
> > probably not the actual data (the two may be the same for small strings
> > if the small value optimisations are used, but who knows?).
>
> > >    file.close();
> > > }
>
> > > void load(struct record *i)
> > > {
> > >    fstream                 file;
>
> > >    file.open(NAME, fstream::binary | fstream::in);
>
> > >    if (!file.is_open())
> > >            throw string("Unable to open file");
>
> > >            file.read((char*) i, sizeof(struct record));    // error
>
> > This will write some random garbage over the strings, causing all sorts
> > of errors when they are deleted.
>
> > If you are going to serialise a struct, you have to serialise each
> > member.  What you are doing could work for a POD struct, but it would be
> > unsafe even for those.
>
> Agreed.  More info here -- http://en.wikipedia.org/wiki/Serialization

C++ FAQ Lite has whole section [36] about serialization.

[toc] | [prev] | [next] | [standalone]

#6230

Ramon <ramif_47.invalid@yahoo.co.uk> wrote:
> Though I have a good amount of experience in ANSI C, Java, C#...etc, I 
> am fairly new with C++.
> 
> What I am trying to do is to persist a struct to a binary file.  The 
> structure is being saved correctly yet when I am trying to load it from 
> disk to memory Valgrind is popping up some errors (Invalid write and 
> Invalid free) from this line of code:
> 
> file.read((char*) i, sizeof(struct record));

  Suppose you had this:

struct record
{
    char* name;
    char* surname;
};

  Would you expect to be able to save and load the contents of that struct
with a simple fwrite() and fread()? If you did that, you would just have
the pointers pointing to garbage (and if you are lucky you will get a
segmentation fault, but if you aren't, your program will just misbehave
in wild ways).

  If you don't expect it to work in C, why would you expect it to work
in C++? Just because the pointers are wrapped into classes (in this case
std::string) doesn't make it any safer to use raw byte writes and reads.

  Unfortunately C++ does not have automatic serialization features like
many other languages do.

[toc] | [prev] | [next] | [standalone]

#6231

"Juha Nieminen" <nospam@thanks.invalid> ha scritto nel messaggio
news:4deb1ab9$0$2827$7b1e8fa0@news.nbl.fi...
> Ramon <ramif_47.invalid@yahoo.co.uk> wrote:
>> Though I have a good amount of experience in ANSI C, Java, C#...etc, I
>> am fairly new with C++.
>>
>> What I am trying to do is to persist a struct to a binary file.  The
>> structure is being saved correctly yet when I am trying to load it from
>> disk to memory Valgrind is popping up some errors (Invalid write and
>> Invalid free) from this line of code:
>>
>> file.read((char*) i, sizeof(struct record));
>
>  Suppose you had this:
>
> struct record
> {
>    char* name;
>    char* surname;
> };
>
>  Would you expect to be able to save and load the contents of that struct
> with a simple fwrite() and fread()? If you did that, you would just have
> the pointers pointing to garbage (and if you are lucky you will get a
> segmentation fault, but if you aren't, your program will just misbehave
> in wild ways).
>
>  If you don't expect it to work in C, why would you expect it to work
> in C++? Just because the pointers are wrapped into classes (in this case
> std::string) doesn't make it any safer to use raw byte writes and reads.
>
>  Unfortunately C++ does not have automatic serialization features like
> many other languages do.

the above not work but something like below should be ok
struct record{
    char  name[100];
    char  surname[100];
 };

struct record  i;
file.read((char*)&i, sizeof(struct record));

[toc] | [prev] | [next] | [standalone]

#6233

On 06/05/2011 08:14 AM, io_x wrote:
> the above not work but something like below should be ok
> struct record{
>      char  name[100];
>      char  surname[100];
>   };
>
> struct record  i;
> file.read((char*)&i, sizeof(struct record));

I have tried that before, yet strangley enough Valgrind still pops out 
an error:  When you try to write the data structure it states that 
"Syscall param write(buf) points to uninitialised byte(s)", i.e. struct 
record *r has not been initialised... strangely enough...

Also, there are not reading errors.

[toc] | [prev] | [next] | [standalone]

#6235

"Ramon" <ramif_47.invalid@yahoo.co.uk> ha scritto nel messaggio 
news:isfiuh$klm$1@dont-email.me...
> On 06/05/2011 08:14 AM, io_x wrote:
>> the above not work but something like below should be ok
>> struct record{
>>      char  name[100];
>>      char  surname[100];
>>   };
>>
>> struct record  i;
>> file.read((char*)&i, sizeof(struct record));
>
> I have tried that before, yet strangley enough Valgrind still pops out an 
> error:  When you try to write the data structure it states that "Syscall param 
> write(buf) points to uninitialised byte(s)", i.e. struct record *r has not 
> been initialised... strangely enough...

if the file has the data for that struct, the record i is initilizated
afther that read

> Also, there are not reading errors. 

[toc] | [prev] | [next] | [standalone]

#6244

Ok, the Valgrind's warning was eliminated by this line of code:

	memset((void *)r, 0, sizeof(struct record));

Thx everyone for your precious help.

[toc] | [prev] | [next] | [standalone]

#6245

Hi Ramon

On the: 05. of juni-2011 At: 18:15 Ramon wrote:

> Ok, the Valgrind's warning was eliminated by this line of code:
>
> 	memset((void *)r, 0, sizeof(struct record));
>
> Thx everyone for your precious help.

if You need to cast it when using memset it is probably
because You need to to take the address of Your record:

memset(&r, 0, sizeof(record));
at least my borland compiler never wanted me to do it.

but that can also be done like this

{
record r = {0};
//fill it
}
but if You create it with new You do need to use memset
record *r = new record();
memset(r, 0, sizeof(record));

but dont use memset or the above if Your struct contains
object like std:string

Best regards
Asger-P

[toc] | [prev] | [next] | [standalone]

#6327

Ramon <ramif_47.invalid@yahoo.co.uk> wrote:
> Ok, the Valgrind's warning was eliminated by this line of code:
> 
>        memset((void *)r, 0, sizeof(struct record));

  I find that strange. I would expect valgrind to go mad rather than 
shutting up. Because you are force-casting an object to a void pointer, 
which means that the resulting pointer will point to some random location 
in memory, and you are writing there. In many cases you should get a 
segmentation fault, but at least valgrind should be giving you errors.

  Remember: If you are getting a type-related compiler error, getting rid 
of the error with a C style cast is usually the wrong solution. You get 
rid of the error, but your program will crash.

[toc] | [prev] | [next] | [standalone]

#6334 — Re: Persisting a structure: Valgrind errors

On Jun 6, 11:20 pm, Juha Nieminen <nos...@thanks.invalid> wrote:
> Ramon <ramif_47.inva...@yahoo.co.uk> wrote:
> > Ok, the Valgrind's warning was eliminated by this line of code:
>
> >        memset((void *)r, 0, sizeof(struct record));
>
>   I find that strange. I would expect valgrind to go mad rather than
> shutting up. Because you are force-casting an object to a void pointer,
> which means that the resulting pointer will point to some random location
> in memory, and you are writing there. In many cases you should get a
> segmentation fault, but at least valgrind should be giving you errors.
>
>   Remember: If you are getting a type-related compiler error, getting rid
> of the error with a C style cast is usually the wrong solution. You get
> rid of the error, but your program will crash.

I am totally lost by this line of reasoning. First, let me note that
as I see it, "r"'s declaration is:
        struct record *r = new struct record;
That is, "r" is a pointer object. The first problem with your post is
that the C-style cast is superfluous. It would compile with or without
it. All pointers are convertible to void pointer implicitly. Second,
why should that cast result in a random location in memory? There is
an important and explicit guarantee that void pointers can round-trip
store any other pointer type. This is required for large amounts of C
code to function.

[toc] | [prev] | [next] | [standalone]

#6338 — Re: Persisting a structure: Valgrind errors

Joshua Maurice <joshuamaurice@gmail.com> wrote:
> On Jun 6, 11:20 pm, Juha Nieminen <nos...@thanks.invalid> wrote:
>> Ramon <ramif_47.inva...@yahoo.co.uk> wrote:
>> > Ok, the Valgrind's warning was eliminated by this line of code:
>>
>> >        memset((void *)r, 0, sizeof(struct record));
>>
>>   I find that strange. I would expect valgrind to go mad rather than
>> shutting up. Because you are force-casting an object to a void pointer,
>> which means that the resulting pointer will point to some random location
>> in memory, and you are writing there. In many cases you should get a
>> segmentation fault, but at least valgrind should be giving you errors.
>>
>>   Remember: If you are getting a type-related compiler error, getting rid
>> of the error with a C style cast is usually the wrong solution. You get
>> rid of the error, but your program will crash.
> 
> I am totally lost by this line of reasoning. First, let me note that
> as I see it, "r"'s declaration is:
>        struct record *r = new struct record;
> That is, "r" is a pointer object. The first problem with your post is
> that the C-style cast is superfluous. It would compile with or without
> it. All pointers are convertible to void pointer implicitly. Second,
> why should that cast result in a random location in memory? There is
> an important and explicit guarantee that void pointers can round-trip
> store any other pointer type. This is required for large amounts of C
> code to function.

  Since you used an explicit cast to void*, which would be unnecessary if 
'r' would have been a pointer, I assumed that you were making the cast 
because 'r' was actually an object and you were getting a compiler error 
and you figured that you would get rid of it by using the cast. In other 
words, you were doing:

    record r;
    memset((void *)r, 0, sizeof(struct record));

  (This actually shows a problem with explicit C style casts: If the 
situation had indeed been like that, the cast would have masked the 
obvious problem and circumvented the type safety mechanisms in the 
language.)

  Which brings me to another thing:

  Your code is very C-like, even though you are programming in C++. 
Firstly, you don't need to litter your code with the 'struct' keyword 
because it's superfluous. Secondly, and more importantly, that's not how 
you initialize a struct/class in C++ style. You do that by using a 
constructor. In other words:

    struct record
    {
        Sometype anArray[123];

        record()
        {
            // initialize anArray here
        }
    };

  (And no, that will not make your program any slower or consume any more 
memory.)

  Thirdly, in general you should avoid allocating objects directly with 
'new' and to a raw pointer unless it's absolutely necessary and you know 
what you are doing. Not only is it hazardous and error-prone, it's also 
inefficient.

  (If the lifetime of the object is limited to the scope where it's 
created, then it should definitely be stack-allocated. If you need eg. an 
array of such objects, use std::vector<record> or other such containers. 
If you really, really must allocate individual objects dynamically, at 
least either use a smart pointer or wrap the management of said objects 
inside a properly designed class.)

[toc] | [prev] | [next] | [standalone]

#6391 — Re: Persisting a structure: Valgrind errors

Juha Nieminen <nospam@thanks.invalid> writes:
>   Since you used an explicit cast to void*, which would be unnecessary if 
> 'r' would have been a pointer

That's true in C, but not in C++.  C++ doesn't implicitly convert
to/from void*.

-Miles

-- 
On a bad day, I see brewers still talking as though all beer were
consumed in the pub, by the pint -- by insatiably thirsty Yorkshire
steelworkers who have in reality long ago sought work as striptease
artists.  [Michael Jackson]

[toc] | [prev] | [next] | [standalone]

#6392 — Re: Persisting a structure: Valgrind errors

On Jun 7, 10:43 pm, Miles Bader <mi...@gnu.org> wrote:
> Juha Nieminen <nos...@thanks.invalid> writes:
> >   Since you used an explicit cast to void*, which would be unnecessary if
> > 'r' would have been a pointer
>
> That's true in C, but not in C++.  C++ doesn't implicitly convert
> to/from void*.

False. In C++, all pointers to objects implicitly convert to void
pointer.
http://www.comeaucomputing.com/tryitout/
  int* x = 0;
  void* y = x;

[toc] | [prev] | [next] | [standalone]

#6399 — Re: Persisting a structure: Valgrind errors

Joshua Maurice <joshuamaurice@gmail.com> writes:
>> That's true in C, but not in C++.  C++ doesn't implicitly convert
>> to/from void*.
>
> In C++, all pointers to objects implicitly convert to void pointer.

Ah, you are right -- it's only the _from_ void* conversion that C++ omits.

-Miles

-- 
[|nurgle|]  ddt- demonic? so quake will have an evil kinda setting? one that
            will  make every christian in the world foamm at the mouth?
[iddt]      nurg, that's the goal

[toc] | [prev] | [next] | [standalone]

#6246

On Sun, 05 Jun 2011 11:40:32 +0200, Ramon wrote:

>> the above not work but something like below should be ok
>> struct record{
>>      char  name[100];
>>      char  surname[100];
>>   };
>>
>> struct record  i;
>> file.read((char*)&i, sizeof(struct record));
> 
> I have tried that before, yet strangley enough Valgrind still pops out 
> an error:  When you try to write the data structure it states that 
> "Syscall param write(buf) points to uninitialised byte(s)", i.e. struct 
> record *r has not been initialised... strangely enough...

This makes sense, as you are passing unpredictable data out of the
process. This can be considered a security flaw, as you may be making
"sensitive" data public.

[toc] | [prev] | [next] | [standalone]

#6250 — Re: Persisting a structure: Valgrind errors

On Jun 5, 12:57 am, Juha Nieminen <nos...@thanks.invalid> wrote:

>   Unfortunately C++ does not have automatic serialization features like
> many other languages do.

I think what C++ needs is on line code generation.
I'm working on that with the C++ Middleware Writer.
(A weakness of the C++ Middleware Writer is limited
support of typedefs.  Thankfully, it looks like I'm
going to be able to improve that support this month.
If you have examples of typedefs used in a marshalling
context, please post them here or send them to me.
Thanks in advance.)


Brian Wood
Ebenezer Enterprises
http://webEbenezer.net

[toc] | [prev] | [next] | [standalone]

#6234

Hi Ramon

On the: 04. of juni-2011 At: 23:19 Ramon wrote:

> Hello,
>
> Though I have a good amount of experience in ANSI C, Java, C#...etc, I  
> am fairly new with C++.

In C++ You need to think more in letting the object handle
its own data.


#include <iostream>
#include <fstream>
#include <string>

using namespace std;

struct record
{
    string name;
    string surname;

    void saveToStream( fstream &strm )
    {
       strm << name << endl;
       strm << surname << endl;
    }
    void readFromStream( fstream &strm )
    {
       getline( strm, name );
       getline( strm, surname );
    }
};


I dont use STL much, so I dont know if this is the best way,
but I'm sure You get the picture


Best regards
Asger-P

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c++

csiph-web