public inbox for gcc-help@gcc.gnu.org
 help / color / mirror / Atom feed
* string in structure and fread()
@ 2011-12-25 22:14 Mohsen Pahlevanzadeh
  2011-12-25 23:05 ` Sam Varshavchik
  2011-12-26  0:59 ` Juan Ramírez
  0 siblings, 2 replies; 6+ messages in thread
From: Mohsen Pahlevanzadeh @ 2011-12-25 22:14 UTC (permalink / raw)
  To: gcc-help

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

Dear all,
I have a program that it should write a struct in binary file.My struct
is here:
///////////////////
struct book{
    string name;
	string author;
	string publisher;
	string translator;
	bool translation;
	bool stock;
};
////////////////////////
Then i wrote the following function that write to file:
////////////////////////////
void WriteFile::addNode(string name, string publisher, string author,
bool stock,string translator , bool translation ){
	book * tmp = new book;
	int line = 1;
	short unsigned i;
	string memory;

	short unsigned size = 100 - strlen((const char *)name.data());
	memory = '`';
	for (i =0 ;i<size;i++)
		memory += '`';
	tmp->name = name  + memory;

	size = 100 - strlen((const char *)publisher.data());
	memory = '`';
	for (i =0 ;i<size;i++)
		memory += '`';
	tmp->publisher =  publisher  + memory;

	size = 100 - strlen((const char *)author.data());
	memory = '`';
	for (i =0 ;i<size;i++)
		memory += '`';
	tmp->author = author  + memory;


	size = 100 - strlen((const char *)translator.data());
	memory = '`';
	for (i =0 ;i<size;i++)
		memory += '`';
	tmp->translator = translator + memory;

	tmp->stock = stock;
	tmp->translation = translation;

	fseek(bookFilePtr,0L,SEEK_END);
	fwrite(ptr,408,1,bookFilePtr);


///////////////////////////
Note that i wanted to use fixed length of each field, So, for example
if a field has 30 character, i fill 71 character "`" at end of field. 30
+71= 101
So 4*string*101+2*bool*4=408 = length of struct.

Then i wrote the following function to  read all of records:

///////////////////////////////////////////////
void ReadFile::printList(){
	fseek(bookFilePtr,0L,SEEK_SET); // set to begin of file


	int counter = 1;
	long int line = 1;
	int pageCounter = 1;


	while ( fread(bookPtrObj,408,1,bookFilePtr) == 1 ){
		string output;
		mvprintw(++line, 27,"***Title*****************Value*********" );
		output = "Name:               " + bookPtrObj->name;
		mvprintw(++line, 27, output.data());

		output = "Publisher:          " + bookPtrObj->publisher;
		mvprintw(++line, 27,output.data());

		output = "Author:             " + bookPtrObj->author;
		mvprintw(++line, 27,output.data());

		output = "Translator:         " + bookPtrObj->translator;
		if (bookPtrObj->translation == true )
			mvprintw(++line, 27,output.data());

		if (bookPtrObj->stock != true )
			mvprintw(++line, 27,"Stock:              The given book doesn't
exist.");
		else
			mvprintw(++line, 27,"Stock:              The given book exist.");

		if ( pageCounter % 3 == 0){
			mvprintw(++line, 27,"Press any key to see next page...");
			getch();
			clear();
			line = 1;

		}
		pageCounter++;
		refresh();

		fseek(bookFilePtr, counter * sizeof(struct book) ,SEEK_SET); // seek
to next data
		counter ++;
	}
}

///////////////////////////////////////////////

But i get the segmentation fault on read each field.
Because i didn't specify length of each field in read.How i specify
length of each field in reading?

--mohsen


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: string in structure and fread()
  2011-12-25 22:14 string in structure and fread() Mohsen Pahlevanzadeh
@ 2011-12-25 23:05 ` Sam Varshavchik
  2011-12-26  0:59 ` Juan Ramírez
  1 sibling, 0 replies; 6+ messages in thread
From: Sam Varshavchik @ 2011-12-25 23:05 UTC (permalink / raw)
  To: gcc-help

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

Mohsen Pahlevanzadeh writes:

> 	short unsigned size = 100 - strlen((const char *)name.data());

std::string::data() is not guaranteed to be null-terminated. Undefined  
behavior. Furthermore, this is overdoing it. name.size() will work much  
better.

> 	memory = '`';
> 	for (i =0 ;i<size;i++)
> 		memory += '`';
> 	tmp->name = name  + memory;

You should replace all of the above effort with a simple:

tmp->name = name;
tmp->name.resize(100, '`');

>
> 	size = 100 - strlen((const char *)publisher.data());
> 	memory = '`';
> 	for (i =0 ;i<size;i++)
> 		memory += '`';
> 	tmp->publisher =  publisher  + memory;

Same here.

tmp->publisher=publisher;
tmp->publisher.resize(100, '`');

> 	fseek(bookFilePtr,0L,SEEK_END);
> 	fwrite(ptr,408,1,bookFilePtr);

Undefined behavior. std::string is not an char[]. It is a structure.

Hint: if you take a look at what sizeof(ptr) tells you, it's not going to be  
408 bytes.


> 	while ( fread(bookPtrObj,408,1,bookFilePtr) == 1 ){

>
> But i get the segmentation fault on read each field.

fread() and fwrite() read and write a raw char buffer. char[]. std::string  
is not a char[]. Internally, std::string is a single pointer to somewhere  
else in memory, where std::string happens to have dumped the current  
contents of your string. If you fread() and fwrite() the std::string itself,  
fread() and fwrite() knows nothing about what it's reading or writing,  
besides the actual bytes themselves. fread() and fwrite() has no knowledge  
that those bytes consist of a pointer to somewhere else in memory, where a  
string is.

If you actually look at what your fwrite() written out, it's binary garbage.

To do this properly, you have to manually place everything you want to write  
out into a single char buffer. std::vector<char> would be an excellent  
choice:

std::vector<char> buffer;

Then, after you filled this buffer with whatever you want to write:

fwrite(&buffer[0], buffer.size(), 1, fp);

But, if you really want to use C++, rather than C, you should use  
std::ofstream, instead of FILE *.

Then to read what you previous wrote out, you have to allocate the buffer  
first, using resize(), then you can read what you want to read, into the  
newly-allocated buffer.


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: string in structure and fread()
  2011-12-25 22:14 string in structure and fread() Mohsen Pahlevanzadeh
  2011-12-25 23:05 ` Sam Varshavchik
@ 2011-12-26  0:59 ` Juan Ramírez
  2011-12-26 11:37   ` Mohsen Pahlevanzadeh
  2011-12-26 11:57   ` Jonathan Wakely
  1 sibling, 2 replies; 6+ messages in thread
From: Juan Ramírez @ 2011-12-26  0:59 UTC (permalink / raw)
  To: Mohsen Pahlevanzadeh; +Cc: gcc-help

Hi,

If you want to write and read a raw structure to a file, using c++
objects is NOT recommended at all! At least not using fread/fwrite

Given that fields have fixed length, a good solution could be using a
raw char array, something like this:

    #define FIELD_LENGTH   101

    struct book {
        char name[FIELD_LENGTH];
        char author[FIELD_LENGTH];
        char publisher[FIELD_LENGTH];
        char translator[FIELD_LENGTH];
        bool translation;
        bool stock;
    };


    // Write

    book mybook;

    fwrite(&mybook, sizeof(book), 1, filePtr);


    // Read

    book mybook;

    fread(&mybook, sizeof(book), 1, filePtr);


It will work awesomely, i promise!

Saludos!
     Juan




On Sun, Dec 25, 2011 at 11:01 PM, Mohsen Pahlevanzadeh
<mohsen@pahlevanzadeh.org> wrote:
>
> Dear all,
> I have a program that it should write a struct in binary file.My struct
> is here:
> ///////////////////
> struct book{
>    string name;
>        string author;
>        string publisher;
>        string translator;
>        bool translation;
>        bool stock;
> };
> ////////////////////////
> Then i wrote the following function that write to file:
> ////////////////////////////
> void WriteFile::addNode(string name, string publisher, string author,
> bool stock,string translator , bool translation ){
>        book * tmp = new book;
>        int line = 1;
>        short unsigned i;
>        string memory;
>
>        short unsigned size = 100 - strlen((const char *)name.data());
>        memory = '`';
>        for (i =0 ;i<size;i++)
>                memory += '`';
>        tmp->name = name  + memory;
>
>        size = 100 - strlen((const char *)publisher.data());
>        memory = '`';
>        for (i =0 ;i<size;i++)
>                memory += '`';
>        tmp->publisher =  publisher  + memory;
>
>        size = 100 - strlen((const char *)author.data());
>        memory = '`';
>        for (i =0 ;i<size;i++)
>                memory += '`';
>        tmp->author = author  + memory;
>
>
>        size = 100 - strlen((const char *)translator.data());
>        memory = '`';
>        for (i =0 ;i<size;i++)
>                memory += '`';
>        tmp->translator = translator + memory;
>
>        tmp->stock = stock;
>        tmp->translation = translation;
>
>        fseek(bookFilePtr,0L,SEEK_END);
>        fwrite(ptr,408,1,bookFilePtr);
>
>
> ///////////////////////////
> Note that i wanted to use fixed length of each field, So, for example
> if a field has 30 character, i fill 71 character "`" at end of field. 30
> +71= 101
> So 4*string*101+2*bool*4=408 = length of struct.
>
> Then i wrote the following function to  read all of records:
>
> ///////////////////////////////////////////////
> void ReadFile::printList(){
>        fseek(bookFilePtr,0L,SEEK_SET); // set to begin of file
>
>
>        int counter = 1;
>        long int line = 1;
>        int pageCounter = 1;
>
>
>        while ( fread(bookPtrObj,408,1,bookFilePtr) == 1 ){
>                string output;
>                mvprintw(++line, 27,"***Title*****************Value*********" );
>                output = "Name:               " + bookPtrObj->name;
>                mvprintw(++line, 27, output.data());
>
>                output = "Publisher:          " + bookPtrObj->publisher;
>                mvprintw(++line, 27,output.data());
>
>                output = "Author:             " + bookPtrObj->author;
>                mvprintw(++line, 27,output.data());
>
>                output = "Translator:         " + bookPtrObj->translator;
>                if (bookPtrObj->translation == true )
>                        mvprintw(++line, 27,output.data());
>
>                if (bookPtrObj->stock != true )
>                        mvprintw(++line, 27,"Stock:              The given book doesn't
> exist.");
>                else
>                        mvprintw(++line, 27,"Stock:              The given book exist.");
>
>                if ( pageCounter % 3 == 0){
>                        mvprintw(++line, 27,"Press any key to see next page...");
>                        getch();
>                        clear();
>                        line = 1;
>
>                }
>                pageCounter++;
>                refresh();
>
>                fseek(bookFilePtr, counter * sizeof(struct book) ,SEEK_SET); // seek
> to next data
>                counter ++;
>        }
> }
>
> ///////////////////////////////////////////////
>
> But i get the segmentation fault on read each field.
> Because i didn't specify length of each field in read.How i specify
> length of each field in reading?
>
> --mohsen
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: string in structure and fread()
  2011-12-26  0:59 ` Juan Ramírez
@ 2011-12-26 11:37   ` Mohsen Pahlevanzadeh
  2011-12-26 11:57   ` Jonathan Wakely
  1 sibling, 0 replies; 6+ messages in thread
From: Mohsen Pahlevanzadeh @ 2011-12-26 11:37 UTC (permalink / raw)
  To: Juan Ramírez; +Cc: gcc-help

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

Hi ,

I read email's Sam carefully, but i didn't test yet.Do you think if i
implement same Sam i can implement string obj in structure?

--mohsen

On Sun, 2011-12-25 at 21:04 -0200, Juan Ramírez wrote:
> Hi,
> 
> If you want to write and read a raw structure to a file, using c++
> objects is NOT recommended at all! At least not using fread/fwrite
> 
> Given that fields have fixed length, a good solution could be using a
> raw char array, something like this:
> 
>     #define FIELD_LENGTH   101
> 
>     struct book {
>         char name[FIELD_LENGTH];
>         char author[FIELD_LENGTH];
>         char publisher[FIELD_LENGTH];
>         char translator[FIELD_LENGTH];
>         bool translation;
>         bool stock;
>     };
> 
> 
>     // Write
> 
>     book mybook;
> 
>     fwrite(&mybook, sizeof(book), 1, filePtr);
> 
> 
>     // Read
> 
>     book mybook;
> 
>     fread(&mybook, sizeof(book), 1, filePtr);
> 
> 
> It will work awesomely, i promise!
> 
> Saludos!
>      Juan
> 
> 
> 
> 
> On Sun, Dec 25, 2011 at 11:01 PM, Mohsen Pahlevanzadeh
> <mohsen@pahlevanzadeh.org> wrote:
> >
> > Dear all,
> > I have a program that it should write a struct in binary file.My struct
> > is here:
> > ///////////////////
> > struct book{
> >    string name;
> >        string author;
> >        string publisher;
> >        string translator;
> >        bool translation;
> >        bool stock;
> > };
> > ////////////////////////
> > Then i wrote the following function that write to file:
> > ////////////////////////////
> > void WriteFile::addNode(string name, string publisher, string author,
> > bool stock,string translator , bool translation ){
> >        book * tmp = new book;
> >        int line = 1;
> >        short unsigned i;
> >        string memory;
> >
> >        short unsigned size = 100 - strlen((const char *)name.data());
> >        memory = '`';
> >        for (i =0 ;i<size;i++)
> >                memory += '`';
> >        tmp->name = name  + memory;
> >
> >        size = 100 - strlen((const char *)publisher.data());
> >        memory = '`';
> >        for (i =0 ;i<size;i++)
> >                memory += '`';
> >        tmp->publisher =  publisher  + memory;
> >
> >        size = 100 - strlen((const char *)author.data());
> >        memory = '`';
> >        for (i =0 ;i<size;i++)
> >                memory += '`';
> >        tmp->author = author  + memory;
> >
> >
> >        size = 100 - strlen((const char *)translator.data());
> >        memory = '`';
> >        for (i =0 ;i<size;i++)
> >                memory += '`';
> >        tmp->translator = translator + memory;
> >
> >        tmp->stock = stock;
> >        tmp->translation = translation;
> >
> >        fseek(bookFilePtr,0L,SEEK_END);
> >        fwrite(ptr,408,1,bookFilePtr);
> >
> >
> > ///////////////////////////
> > Note that i wanted to use fixed length of each field, So, for example
> > if a field has 30 character, i fill 71 character "`" at end of field. 30
> > +71= 101
> > So 4*string*101+2*bool*4=408 = length of struct.
> >
> > Then i wrote the following function to  read all of records:
> >
> > ///////////////////////////////////////////////
> > void ReadFile::printList(){
> >        fseek(bookFilePtr,0L,SEEK_SET); // set to begin of file
> >
> >
> >        int counter = 1;
> >        long int line = 1;
> >        int pageCounter = 1;
> >
> >
> >        while ( fread(bookPtrObj,408,1,bookFilePtr) == 1 ){
> >                string output;
> >                mvprintw(++line, 27,"***Title*****************Value*********" );
> >                output = "Name:               " + bookPtrObj->name;
> >                mvprintw(++line, 27, output.data());
> >
> >                output = "Publisher:          " + bookPtrObj->publisher;
> >                mvprintw(++line, 27,output.data());
> >
> >                output = "Author:             " + bookPtrObj->author;
> >                mvprintw(++line, 27,output.data());
> >
> >                output = "Translator:         " + bookPtrObj->translator;
> >                if (bookPtrObj->translation == true )
> >                        mvprintw(++line, 27,output.data());
> >
> >                if (bookPtrObj->stock != true )
> >                        mvprintw(++line, 27,"Stock:              The given book doesn't
> > exist.");
> >                else
> >                        mvprintw(++line, 27,"Stock:              The given book exist.");
> >
> >                if ( pageCounter % 3 == 0){
> >                        mvprintw(++line, 27,"Press any key to see next page...");
> >                        getch();
> >                        clear();
> >                        line = 1;
> >
> >                }
> >                pageCounter++;
> >                refresh();
> >
> >                fseek(bookFilePtr, counter * sizeof(struct book) ,SEEK_SET); // seek
> > to next data
> >                counter ++;
> >        }
> > }
> >
> > ///////////////////////////////////////////////
> >
> > But i get the segmentation fault on read each field.
> > Because i didn't specify length of each field in read.How i specify
> > length of each field in reading?
> >
> > --mohsen
> >


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: string in structure and fread()
  2011-12-26  0:59 ` Juan Ramírez
  2011-12-26 11:37   ` Mohsen Pahlevanzadeh
@ 2011-12-26 11:57   ` Jonathan Wakely
  2011-12-26 20:01     ` Juan Ramírez
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2011-12-26 11:57 UTC (permalink / raw)
  To: Juan Ramírez; +Cc: Mohsen Pahlevanzadeh, gcc-help

2011/12/25 Juan Ramírez:
> Hi,
>
> If you want to write and read a raw structure to a file, using c++
> objects is NOT recommended at all! At least not using fread/fwrite

What do you mean by "c++ objects"?

A POD struct is a "c++ object"

Assuming you mean non-POD class types with non-trivial constructors
and/or non-POD members, I strongly disagree, there is nothing wrong
with doing IO with suh types, you just need to write the IO code
correctly, which is always true.


> Given that fields have fixed length, a good solution could be using a
> raw char array, something like this:
>
>     #define FIELD_LENGTH   101
>
>     struct book {
>         char name[FIELD_LENGTH];
>         char author[FIELD_LENGTH];
>         char publisher[FIELD_LENGTH];
>         char translator[FIELD_LENGTH];
>         bool translation;
>         bool stock;
>     };

That solution is useless if the strings can be arbitrarily long, a
better solution is to store the strings in the file as
null-terminated, or length-prefixed, and in either of those cases
managing the strings in the program will be simpler with std::string.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: string in structure and fread()
  2011-12-26 11:57   ` Jonathan Wakely
@ 2011-12-26 20:01     ` Juan Ramírez
  0 siblings, 0 replies; 6+ messages in thread
From: Juan Ramírez @ 2011-12-26 20:01 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Mohsen Pahlevanzadeh, gcc-help

> What do you mean by "c++ objects"?>> A POD struct is a "c++ object"

of course, that's out of the question here

> Assuming you mean non-POD class types with non-trivial constructors
> and/or non-POD members, I strongly disagree, there is nothing wrong
> with doing IO with suh types, you just need to write the IO code
> correctly, which is always true

I know you are right, but looking at Mohsen's code I can tell that he
has not a C++ guru, and getting into the that could lead to a lot of
problems.
In order to use c++ string he should resize the string to the fixed
length of 101 and then perform a call to fread using &str[0] (to treat
is as an array), the same behavior if vector<char> is used.

> That solution is useless if the strings can be arbitrarily long, a
> better solution is to store the strings in the file as
> null-terminated, or length-prefixed, and in either of those cases
> managing the strings in the program will be simpler with std::string.

Well, I made that only because Mohsen said strings have fixed length.

Regards


Saludos!
     Juan




2011/12/26 Jonathan Wakely <jwakely.gcc@gmail.com>:
> 2011/12/25 Juan Ramírez:
>> Hi,
>>
>> If you want to write and read a raw structure to a file, using c++
>> objects is NOT recommended at all! At least not using fread/fwrite
>
> What do you mean by "c++ objects"?
>
> A POD struct is a "c++ object"
>
> Assuming you mean non-POD class types with non-trivial constructors
> and/or non-POD members, I strongly disagree, there is nothing wrong
> with doing IO with suh types, you just need to write the IO code
> correctly, which is always true.
>
>
>> Given that fields have fixed length, a good solution could be using a
>> raw char array, something like this:
>>
>>     #define FIELD_LENGTH   101
>>
>>     struct book {
>>         char name[FIELD_LENGTH];
>>         char author[FIELD_LENGTH];
>>         char publisher[FIELD_LENGTH];
>>         char translator[FIELD_LENGTH];
>>         bool translation;
>>         bool stock;
>>     };
>
> That solution is useless if the strings can be arbitrarily long, a
> better solution is to store the strings in the file as
> null-terminated, or length-prefixed, and in either of those cases
> managing the strings in the program will be simpler with std::string.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-12-26 16:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-25 22:14 string in structure and fread() Mohsen Pahlevanzadeh
2011-12-25 23:05 ` Sam Varshavchik
2011-12-26  0:59 ` Juan Ramírez
2011-12-26 11:37   ` Mohsen Pahlevanzadeh
2011-12-26 11:57   ` Jonathan Wakely
2011-12-26 20:01     ` Juan Ramírez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).