Hi,
I am writing an array data to a file. The data I am getting is from some sensor and need to be written in real time. I have manged to write it but I am having problem when I try to read it, as I cannot position the pointer to read 'read_ampl'. So three questions:
1. Is this the way I am writing is fast so that I do not drop frames.
2. Am I ok in writing part as I believe I am appending two arrays one after the other.
3. I can read the first array 'read_dist' but cannot read the 'read_ampl' as I am positioning properly.
Any help please.!!!

#include "stdafx.h"
#include <iostream>
#include <stdio.h>
#include<fstream>
#include "sdk.h"
using namespace std;

void WriteToFile(const char *filename,Handle hnd,int ArraySize);
int _tmain(int argc, _TCHAR* argv[])
{
	static const char filename[] = "data.dat";
	int ArraySize;
	Handle hnd; // connection handle
	
    ArraySize=3072;
	WriteToFile(filename, hnd, ArraySize);
	return 0;
}
void WriteToFile(const char *filename, Handle hnd, int ArraySize)
{
	double read_dist[3072], read_ampl[3072];
	int res,i;
	double *dist, *ampl;
	res = Update (hnd);
	if (res != OK) cout<<"No Update";
	
    res = Distances (hnd, (void**)&dist);
	if (res != OK) cout<<"No Distance";

	res = Amplitudes (hnd, (void**)&ampl);
	if (res != OK) cout<<"No Amplitude";

		std::ofstream fileout(filename,ios::out | ios::binary );
			if(!fileout)
		  	{ 
		    cout << "Cannot open file to write.\n"; 
		  	} 
		   fileout.write((char *) dist, 8*ArraySize);  //size of double array
		   fileout.write((char *) ampl, 8*ArraySize);
 		   fileout.close();
	
	ifstream filein(filename, ios::in | ios::binary); 
  if(!filein) {   
	  cout << "Cannot open file to read.\n"; } 

   filein.read((char *) &read_dist, sizeof read_dist); 
   filein.read((char *) &read_ampl, sizeof read_ampl); 
    for(i = 0; i <ArraySize; i++) // show  from file 
		cout << "Distance: "<<read_dist[i] <<endl; 
	    cout << "Amplitude: "<<read_ampl[i] <<endl; 

   filein.close();

}

Is this to be production code? I assume it is so my response might be a little more embarrassingly detailed than you would like...

First, for your main problem. I think you are clobbering memory somewhere --I'm actually surprised your program isn't crashing. Try this:

filein.read((char *) read_dist, sizeof( read_dist )); 
   filein.read((char *) read_ampl, sizeof( read_ampl ));

Remember, read_dist is a pointer to double. You are casting it to a pointer to char. Either way what it points to is the first double in the array, but you were giving the read function a pointer to the pointer to the first double in the array...

Second. the sizeof requires parentheses. It's nice you know a double is eight bytes long, but you should (if just for style) not count on it. Say: sizeof( double ) * ArraySize and sizeof( read_dist ) Third, (and I realize that this is just testing), try to get rid of all the hard-coded stuff and stick to using ArraySize. In your production code use double *read_dist = malloc( sizeof( double ) * ArraySize ); or double *read_dist = new double[ ArraySize ]; to allocate memory for the array. Use free or delete when you are done with it.

Fourth, it is good that you are testing for errors, but there are currently no consequences. For example, if opening the file fails you print out that it did then try to read a huge array anyway. While this code works fine, it does so for the wrong reasons...

Finally, don't use using namespace std;. That should never appear in production code. Take the extra tenth of a second to write std::cout instead of just cout. You'll be avoiding horrible bug-hunting sessions in reruns.

Hope this helps.

Hi, Thanks for your reply. I have simplified my code and have put some dummy data. But by using your code I am not getting the right read values, probably I am getting the address of the pointer. Can you copy paste the code below and see yourself.
Thanks,

#include <iostream>
#include <stdio.h>
#include<fstream>
using namespace std;

void WriteToFile(const char *filename, int ArraySize);
int main()
{
	static const char filename[] = "data.dat";
	int ArraySize=10;
	WriteToFile(filename, ArraySize);
	return 0;
}
void WriteToFile(const char *filename, int ArraySize)
{

	double *read_dist=new double[ArraySize];
	double *read_ampl=new double[ArraySize];
	int res,i;
	double c[10]={1,2,3,4,5,6,7,8,9,10};
	double d[10]={5,6,7,8,9,4,3,2,3,4};
	double *dist=c;
	double *ampl=d;
	
			std::ofstream fileout(filename,ios::out | ios::binary );
			if(!fileout)
		  	{ 
		    std::cout << "Cannot open file to write.\n"; 
		  	} 
		   fileout.write((char *) dist, sizeof(double)*ArraySize);  //size of double with array size
		   fileout.write((char *) ampl, sizeof(double)*ArraySize);
 		   fileout.close();
	
	ifstream filein(filename, ios::in | ios::binary); 
  if(!filein) {   
	  std::cout << "Cannot open file to read.\n"; } 

  filein.read((char *) read_dist, sizeof( read_dist)); 
  filein.read((char *) read_ampl, sizeof( read_ampl)); 
  
   //filein.read((char *) &read_dist, sizeof read_dist); 
   //filein.read((char *) &read_ampl, sizeof read_ampl); 
   
   for(i = 0; i <ArraySize; i++) // show values read from file 
   {
	   std::cout << "Distance: "<<read_dist[i] <<endl; 
	    std::cout << "Amplitude: "<<read_ampl[i] <<endl; 
   }
   filein.close();
   delete [] read_dist;
   delete [] read_ampl;
 }

> Remember, read_dist is a pointer to double.
Funny, it looked like an array to me.

> Second. the sizeof requires parentheses.
Wrong again, it only needs () when it's a variable name, not a type name.

Go back to your original code fmufti


> fileout.write((char *) dist, sizeof(dist[0])*ArraySize); //size of double array
> fileout.write((char *) ampl, sizeof(ampl[0])*ArraySize);

> filein.read((char *) &read_dist, sizeof read_dist);
> filein.read((char *) &read_ampl, sizeof read_ampl);
The reason you're not getting any read_ampl data is that read_dist doesn't know when to stop, so it reads as much as possible until the array is full.
If your data arrives in variable length arrays, then you should write out the length first.

fileout.write((char *)&ArraySize, sizeof(ArraySize));
	   fileout.write((char *) dist, 8*ArraySize);  //size of double array
	   fileout.write((char *) ampl, 8*ArraySize);
	
   filein.read((char *) &newSize, sizeof(newSize) );
   filein.read((char *) read_dist, newSize * sizeof double ); 
   filein.read((char *) read_ampl, newSize * sizeof(read_ampl[0]) );

Wow, you're awake too...

The problem is when you changed read_dist and read_ampl from static arrays to pointers. The sizeof() operator always gives the size of the object indicated. The size of an array is much larger than the size of a pointer.

So, you'll have to change things thus:

filein.read((char *) read_dist, sizeof( double ) * ArraySize ); 
  filein.read((char *) read_ampl, sizeof( double ) * ArraySize );

Sorry for the confusion. I should have warned you about that...

Salem, so far I've only seen you give good advice. So don't be stupid now.

1. Arrays in C/C++ are managed by having a variable that points to the first element in the array. The only difference between a[0] and *a is compiler magic.

2. He was saying sizeof read_dist. In case you didn't notice, read_dist is a variable, not a type.

3. His original code didn't work, hence his taking the time to post here.

4. That is not the reason he is not getting ampl data. If it were just a matter of buffer overflow he'd still get his ampl data since the array is declared in memory immediately following the dist data. It would overflow straight into the correct place.

The reason was that he was giving the address of the pointer to the array data, rather than the address of the array data. Hence, he was reading into his code segment, which Win32 prohibits. Hence, there are two reasons his program should have crashed: Win32 would complain; barring that he would destroy his code before it got to the read_ampl part. Lucky he didn't have some bad opcodes in his dist data.

Good thing: yes, writing out the size of the array is a good idea.

Boy are you ever wrong.

> Arrays in C/C++ are managed by having a variable that points to the first element in the array.
Read THIS VERY CAREFULLY.

void foo ( ) {
    int arr[100];
    int *parr = new int[100];

    // these are VERY different
    std::cout << sizeof(arr) << std::endl;
    std::cout << sizeof(parr) << std::endl;

    // these are functionally the same
    std::cout << (void*)arr << " " << (void*)&arr[0] << std::endl;
    std::cout << (void*)parr << " " << (void*)&parr[0] << std::endl;
}

$ ./a.exe
400
4
0x22cb30 0x22cb30
0x6a0250 0x6a0250

If your argument were true, both sizeof results would be the same.

> His original code didn't work, hence his taking the time to post here.
True, but not for any of the reasons you spouted.

The only problem I could see was that he was trying to write out 10 doubles, then read in 3072 doubles into the first array, thus leaving none for the 2nd array.

>> Second. the sizeof requires parentheses.
>Wrong again, it only needs () when it's a variable name, not a type name.
It's the other way around. sizeof requires parentheses with a type name, but not a variable name.

>1. Arrays in C/C++ are managed by having a variable that points to the first element in the array.
How they're managed is none of your business (ie. it's an implementation detail that isn't specified). What matters is how arrays behave when you use them. And the behavior is that except in three situations, an array name is converted to a pointer to the first element. But that's conceptually very different from an array name being a pointer. The three situations are:

1) As the operand to sizeof
2) As the operand to &
3) As a string literal initializer

>The reason was that he was giving the address of the pointer
>to the array data, rather than the address of the array data.
Both of which are equivalent with his compiler. Given an array a, (a) and (&a) both evaluate to the same address. The only difference is the data type, which is corrected with the cast to char*. The address-of operator is still incorrect, but it produces the correct result. All of that writing to the code segment garbage is wrong.

Thanks to all of you. However, the problem is not fully over, as I mentioned that actual data will from sensor so I need to keep writing the frames till the user press some key e.g 's'.
1. How to do that ?
2. How to put the frame no before each dist and ampl arrays are written.
3. And to read them
I have put my my code as

#include "stdafx.h"
#include<iostream>
#include<stdio.h>
#include <conio.h>
#include<fstream>
using namespace std;


void WriteToFile(const char *filename, int ArraySize);
void ReadFromFile(const char *filename, int ArraySize);
int _tmain(int argc, _TCHAR* argv[])
{
	char what;
	static const char filename[] = "data.dat";
	int ArraySize=10;
	std::cout<<"To Write: 'w' and press 's' to stop, To Read 'r'"<<endl;
	std::cin>>what;
	if (what=='w'){
		WriteToFile(filename, ArraySize);}
	else if (what=='r'){
		ReadFromFile(filename, ArraySize);}
	else exit(1);
	return 0;
}
void WriteToFile(const char *filename, int ArraySize)
{
	double c[10]={1,2,3,4,5,6,7,8,9,10};
	double d[10]={5,6,7,8,9,4,3,2,3,4};
	double *dist=c;
	double *ampl=d;
	char stop;
	
			std::ofstream fileout(filename,ios::out | ios::binary );
			if(!fileout) {std::cout << "Cannot open file to write.\n"; }
		//Some function will provide dist and ampl value for each frame;
			while(kbhit())
			{
 		    fileout.write((char *) dist, sizeof(double)*ArraySize);  
		    fileout.write((char *) ampl, sizeof(double)*ArraySize);
			}
 		    fileout.close();
	
	 }
 void ReadFromFile(const char *filename, int ArraySize)
 {
	int i;
	double *read_dist=new double[ArraySize];
	double *read_ampl=new double[ArraySize];
	 ifstream filein(filename, ios::in | ios::binary); 
		if(!filein) {   
		std::cout << "Cannot open file to read.\n"; } 
		filein.read((char *) read_dist, sizeof( double ) * ArraySize );
		filein.read((char *) read_ampl, sizeof( double ) * ArraySize ); 

     for(i = 0; i <ArraySize; i++) 
	 {
	 std::cout << "Distance: "<<read_dist[i] <<endl; 
	 std::cout << "Amplitude: "<<read_ampl[i] <<endl; 
	 }
   filein.close();
   delete [] read_dist;
   delete [] read_ampl;
 }

Thanks

Well, you people win. Whether or not you like it I generally have a pretty good idea what I am talking about and don't like being chopped at for helping someone out. I can live with honest mistakes (such as with sizeof --I just always used parens). But being contradicted to satisfy your own arrogant egos is too much. If you all had spent any time reading the contents of this thread you'd find a lot less to fault me with.

I hate snot.

Btw. Any C compiler that considers a or &a[0] the same as &a is just waiting to punish people. Understanding how pointers work is very much the business of the programmer --as without that knowledge they'll wind up regurgitating circular crap on some random forum. Go grabbing the address of some pointer variable on the stack and treating it the same as the address the pointer variable contains at your own peril. You might want to study how linkers arrange data into a PE32, and how the windows loader puts it into memory, as well.

@fmufti
How you arrange your data is up to you. If you want to write the frame number as an integer before writing all those doubles then that would work. Just read the data back in in the same order you wrote it out.

Also, put while (!kbhit()) to continue while a key has not been pressed.

Bye.

Hi,

I have changed kbhit to !_kbhit. However, my program is crashing. I did incorporated frame number before as you suggested, but it was not working so I posted it for you. anyway,any idea why its crashing. Also is there any better way then _kbhit ??.

        std::ofstream fileout(filename,ios::out | ios::binary );
                  if(!fileout) {std::cout << "Cannot open file to write.\n"; }
            int frame_nbr=2;
                 while( !_kbhit() )
                              {
                      //Some function will provide dist and ampl value for each frame;
                      fileout.write((char*) frame_nbr, sizeof(int));
                      fileout.write((char*) dist, sizeof(double)*ArraySize);  
                      fileout.write((char*) ampl, sizeof(double)*ArraySize);
                      ++frame_nbr;
                              }
                 fileout.close();
      
       }
 void ReadFromFile(const char *filename, int ArraySize)
 {
      int i;
      double *read_dist=new double[ArraySize];
      int   *read_fnbr=new int[1];
      double *read_ampl=new double[ArraySize];
       ifstream filein(filename, ios::in | ios::binary);
            if(!filein) {  
            std::cout << "Cannot open file to read.\n"; }
            filein.read((char *) read_fnbr, sizeof(int) );
            filein.read((char *) read_dist, sizeof( double ) * ArraySize );
            filein.read((char *) read_ampl, sizeof( double ) * ArraySize );

     for(i = 0; i <ArraySize; i++)
       {
      std::cout << "Frame: "<<read_fnbr[i] <<endl;
      std::cout << "Distance: "<<read_dist[i] <<endl;
       std::cout << "Amplitude: "<<read_ampl[i] <<endl;
       }
   filein.close();
   delete [] read_dist;
   delete [] read_ampl;
 }

@mufti
You are writing out ArraySize items each frame. According to your read function it looks like you want only one dist and one ampl per frame.

Hi Duoas
Yes, I want to since each frame consists of dist and ampl and that is how I have written in first place, so I would like to read it like that. But my problem is while writing the frame number before dist and ampl in writing part my program crashes, so I cannot check the read part till write part is ok :(

Well, you people win. Whether or not you like it I generally have a pretty good idea what I am talking about and don't like being chopped at for helping someone out. I can live with honest mistakes (such as with sizeof --I just always used parens). But being contradicted to satisfy your own arrogant egos is too much. If you all had spent any time reading the contents of this thread you'd find a lot less to fault me with.

I hate snot.

It seems you're too stubborn and full of yourself to take any kind of criticism. People like that don't tend to last very long around here, where there are members of equal or greater knowledge. So we'll enjoy your posts while you stick around, and when you get too fed up with our "snot"[1] you can run away to some other forum where you'll be worshiped by the ignorant masses.

>Any C compiler that considers a or &a[0] the same as &a is just waiting to punish people.
True, though once you cast away the type, it's completely your fault if you get punished. You tell us to read your posts, but why should we when you don't try to comprehend ours?

>Go grabbing the address of some pointer variable on the
>stack and treating it the same as the address the pointer
>variable contains at your own peril.
*sigh* Okay, I'll bite. If your compiler produces the same address for an object and a pointer to the object, which in the case of Visual C++ and an array object is true, only the data type is different. Now, while if you do something like this, you're lucky if it works:

char a[10];
scanf ( "%9s", &a );

That's completely because scanf does what you told it to, and tries to convert char* instead of char(*)[10]. The data type is significant and undefined behavior results. If you cast away the data type difference, there is no difference. For example:

double a[10];
filein.read ( (char*)&a, sizeof a );

If the compiler gives you 0012FF60 for both a and &a, all you're doing is casting 0012FF60 to char*. It's completely irrelevant that 0012FF60 was double(*)[10] before, it's char* now. Likewise, if you omit the address-of operator, it doesn't matter that 0012FF60 was double* before, it's char* now. Both are completely identical.

You can prattle on about addresses, PE32, and linkers and loaders all you want, but I'd love to hear you explain how the same address magically could mean wildly different things despite the data type being the same.

Actually, scratch that. I don't want to hear it. You're clearly paying only enough attention to be dangerous, you're not open to the possibility that you might be wrong, and it seems like you expect us to believe everything you say without question because you're an experienced programmer. Give me a break.

I certainly agree that using the address-of operator in the OP's case was wrong, and the code is non-portable because of it, but it's not what was causing his problems.

And what's this circular crap you keep talking about? I haven't seen any glaring inconsistencies in our arguments.

[1] It seems "snot" to you is any disagreement.

> fileout.write((char*) frame_nbr, sizeof(int));
This should be
fileout.write((char*) &frame_nbr, sizeof(int));

> double *read_dist=new double[ArraySize];
> int *read_fnbr=new int[1];
> double *read_ampl=new double[ArraySize];
If the arraySize is a constant (you always write the same sized array), you may as well just declare them as such, and save a lot of work.

double read_dist[ArraySize];
      int   read_fnbr;
      double read_ampl=[ArraySize];

Don't forget if you do this to add the & to the reading of read_fnbr.

> std::cout << "Frame: "<<read_fnbr <<endl;
frame number is a single int, not an array.
Print this just the once, then loop for the other two values.

Hi,
Its strange that once I read the data the last frame is read twice. It seems pointer of eof() points to the start of the data and not to the end of data. So for this case data is displayed 5 times instead of 4 and the 5th data is the repetition of 4th data ???

void WriteToFile(const char *filename, int ArraySize)
{
	double c[10]={1,2,3,4,5,6,7,8,9,10};
	double d[10]={5,6,7,8,9,4,3,2,3,4};
	double *dist=c;
	double *ampl=d;
	int frame_nbr=0;
	
			std::ofstream fileout(filename,ios::out | ios::binary );
			if(!fileout) {std::cout << "Cannot open file to write.\n"; }
            
           for (int n=0;n<4;n++)
			// while( !_kbhit() )
					{
                      //Some function will provide dist and ampl value for each frame;
                      frame_nbr=frame_nbr+1;
					  fileout.write((char*) &frame_nbr, sizeof(int));
                      fileout.write((char*) dist, sizeof(double)*ArraySize);  
                      fileout.write((char*) ampl, sizeof(double)*ArraySize);
                      
					}
 		    fileout.close();
	
	 }
 void ReadFromFile(const char *filename, int ArraySize)
 {
	int i;
	double *read_dist=new double[ArraySize];
	double *read_ampl=new double[ArraySize];
	int    read_fnbr;
	ifstream filein(filename, ios::in | ios::binary); 
	if(!filein) {   
		std::cout << "Cannot open file to read.\n"; } 
		while (!filein.eof( )) 
		{
		filein.read((char *) &read_fnbr, sizeof(int) );
		filein.read((char *) read_dist, sizeof( double ) * ArraySize );
		filein.read((char *) read_ampl, sizeof( double ) * ArraySize ); 
			std::cout << "Frame: "<<read_fnbr <<endl; 
				for(i = 0; i <ArraySize; i++) 
				{
				std::cout << "Distance: "<<read_dist[i] <<endl; 
				std::cout << "Amplitude: "<<read_ampl[i] <<endl; 
				}
		}
		filein.close();
		delete [] read_dist;
		delete [] read_ampl;
 }

eof() doesn't do what you think it does.
If you read 10 bytes from a file which is 10 bytes long, then eof() will still return FALSE.

You should initially use the return result of the file reading functions to determine whether you got the data you expected, or there was some error.

Apart from that, it's looking OK.

Hi
I want to read the whole file. Can you tell me how to implement what you have suggested in your latest post.
Thanks,

Making the assumption that the file is well structured (a frame number is always followed by frame data) (but you might want to beef this up later on), try

while ( filein.read((char *) &read_fnbr, sizeof(int) ) ) {
	filein.read((char *) read_dist, sizeof( double ) * ArraySize );
	filein.read((char *) read_ampl, sizeof( double ) * ArraySize ); 
	// rest of code
}
if ( ! filein.eof() ) {
  // could be trouble, the loop exited, and it wasn't eof()
}

So if reading a frame is successful, then assume frame data is OK.

If there is no frame number to read (or an error occurs) then the loop exits, and you can (if you wish) determine the cause.

Hi, it works but there will be problem once the frame number is there and the program has been terminated by the user then part of dist or ampl is captured. Won't there be a problem ?

You mean when you add the
while( !_kbhit() )
part of the program?

As written, the only problem I can see is the file system becoming full, which would prevent a complete record from being written.

In the end, it's all about making sure that the writer writes enough information to enable the reader to do it's thing. After that, it's just making sure they're consistent.

Thanks, but the problem of not complete frame is still not resolved, however, I can leave it here for the time being. I just need to ask you one more thing that I have noticed and that is regarding the size of binary file. Initially I believed that writing to a binary would be better instead of text so that I have small file size. Can you tell me how to calculate the file for 3072 data values for two frames plus a frame number. So that I can calculate if there are 25 fps data coming what would be the size.
As I find the size too big! for few minutes recording. Can you please comment ?

Ok, I would like to rephrase, I know that int will take 4 bytes, and double will take 8 bytes so in this way I can calculate the size, but is there any way I can reduce the size of file. If I ignore the precision level of storing variables so that my file size is smaller. ?

The frame number is tiny compared to the size of everything else, so ignore for rough calcs.
3072 * 8 * 2 = 49152 bytes per frame
*25 per second = 1228800 bytes per second
*60 = 73728000 bytes per minute

Yeah, 70Meg/Min is a fair amount.

Making them floats rather than doubles will basically halve the size.

The original software with that hardware produces only one value that is ampl but writes it in XML format in text value like 3.846 as 3846 and the file size was also small. Do you have any idea to store the decimal precision like that and then at reading time I can covert it back by just dividing the value by 1000.

This article has been dead for over six months. Start a new discussion instead.