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

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

How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++

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

Contents

  How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Hetware <fjolsvit@gmail.com> - 2011-07-15 15:36 -0700
    Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Ian Collins <ian-news@hotmail.com> - 2011-07-16 11:30 +1200
      Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Hetware <fjolsvit@gmail.com> - 2011-07-15 17:23 -0700
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Hetware <fjolsvit@gmail.com> - 2011-07-15 17:58 -0700
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file);   in C++ Ian Collins <ian-news@hotmail.com> - 2011-07-16 13:18 +1200
          Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Hetware <fjolsvit@gmail.com> - 2011-07-15 18:37 -0700
          Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ gwowen <gwowen@gmail.com> - 2011-08-10 03:00 -0700
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Arvind <bharti.arvind@gmail.com> - 2011-07-15 20:39 -0700
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Nobody <nobody@nowhere.com> - 2011-07-16 13:49 +0100
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Todd Carnes <toddcarnes@gmail.com> - 2011-07-16 13:17 +0000
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Paul N <gw7rib@aol.com> - 2011-07-16 09:51 -0700
          Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Paavo Helde <myfirstname@osa.pri.ee> - 2011-07-16 12:16 -0500
    Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Paavo Helde <myfirstname@osa.pri.ee> - 2011-07-16 03:46 -0500
      Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Juha Nieminen <nospam@thanks.invalid> - 2011-07-17 13:46 +0000
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Robert Wessel <robertwessel2@yahoo.com> - 2011-07-17 12:51 -0500
    Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Krice <paulkp@mbnet.fi> - 2011-07-16 02:44 -0700
    Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ hanukas <jukka@liimatta.org> - 2011-07-17 11:32 -0700
      Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Hetware <fjolsvit@gmail.com> - 2011-08-01 09:18 -0700
        Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ Jorgen Grahn <grahn+nntp@snipabacken.se> - 2011-08-01 20:17 +0000
      Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++ James Kanze <james.kanze@gmail.com> - 2011-08-07 11:33 -0700

#7847 — How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++

I have some ancient c code that reads files stored in raw binary
format.  The first 216 bytes are to be read into an instance of:

struct Sheader
{
	long	attr_start;
	long	form_start;
	short	fsize;
	short	attr_size;
	short	lastfieldidx;
	short 	num_inputs;
	char	oldheader[200];
};

The way the C code does this is by fread ((void *)header, sizeof
(struct Sheader), 1, file);  Is there a better way to do this in C++?

[toc] | [next] | [standalone]

#7852

On 07/16/11 10:36 AM, Hetware wrote:
> I have some ancient c code that reads files stored in raw binary
> format.  The first 216 bytes are to be read into an instance of:
>
> struct Sheader
> {
> 	long	attr_start;
> 	long	form_start;
> 	short	fsize;
> 	short	attr_size;
> 	short	lastfieldidx;
> 	short 	num_inputs;
> 	char	oldheader[200];
> };
>
> The way the C code does this is by fread ((void *)header, sizeof
> (struct Sheader), 1, file);  Is there a better way to do this in C++?

Not really (but you don't need 'struct" in there), it's awful in C and 
it will still be awful in C++!

-- 
Ian Collins

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

#7856

On Jul 15, 7:30 pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 07/16/11 10:36 AM, Hetware wrote:

> > I have some ancient c code that reads files stored in raw binary
> > format.  The first 216 bytes are to be read into an instance of:
>
> > struct Sheader
> > {
> >    long    attr_start;
> >    long    form_start;
> >    short   fsize;
> >    short   attr_size;
> >    short   lastfieldidx;
> >    short   num_inputs;
> >    char    oldheader[200];
> > };
>
> > The way the C code does this is by fread ((void *)header, sizeof
> > (struct Sheader), 1, file);  Is there a better way to do this in C++?
>
> Not really (but you don't need 'struct" in there), it's awful in C and
> it will still be awful in C++!
>
> --
> Ian Collins

I'm not sure why struct is "awful".  At this point, its not a question
of whether I like it or not.  I just need a way to get the first bytes
of the file read in as POD.  I actually did make Sheader into a class,
but that really makes little difference.  The following doesn't appear
to get the correct data into the class members:

#include <iostream>

class Header {
public:
  Header(){}
  Header(std::istream& in);
  std::ostream& print(std::ostream& out);
private:
  long	attr_start;
  long	form_start;
  short	fsize;
  short	attr_size;
  short	lastfieldidx;
  short num_inputs;
  char	oldheader[200];

};


Header::Header(std::istream& in) {
  in>>attr_start
    >>form_start
    >>fsize
    >>attr_size
    >>lastfieldidx
    >>num_inputs;
}

std::ostream& Header::print(std::ostream& out) {

  return out
    <<"attr_start   = " << (long)attr_start   << std::endl
    <<"form_start   = " << (long)form_start   << std::endl
    <<"fsize        = " << (short)fsize        << std::endl
    <<"attr_size    = " << (short)attr_size    << std::endl
    <<"lastfieldidx = " << (short)lastfieldidx << std::endl
    <<"num_inputs   = " << (short)num_inputs   << std::endl;

    //oldheader[200];

}

#include <fstream>
#include <sstream>

int main(int argc, char* argv[]) {
  if (argc < 1) { std::cerr<<"enter a file name"<<std::endl; return
-1; }

  std::ifstream inf(argv[1]);
  if(inf) {
    Header header(inf);
    header.print(std::cout);
    return 0;
  }
}

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

#7857

On Jul 15, 8:23 pm, Hetware <fjols...@gmail.com> wrote:
> On Jul 15, 7:30 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
>
>
>
>
>
>
>
>
> > On 07/16/11 10:36 AM, Hetware wrote:
> > > I have some ancient c code that reads files stored in raw binary
> > > format.  The first 216 bytes are to be read into an instance of:
>
> > > struct Sheader
> > > {
> > >    long    attr_start;
> > >    long    form_start;
> > >    short   fsize;
> > >    short   attr_size;
> > >    short   lastfieldidx;
> > >    short   num_inputs;
> > >    char    oldheader[200];
> > > };
>
> > > The way the C code does this is by fread ((void *)header, sizeof
> > > (struct Sheader), 1, file);  Is there a better way to do this in C++?
>
> > Not really (but you don't need 'struct" in there), it's awful in C and
> > it will still be awful in C++!
>
> > --
> > Ian Collins
>
> I'm not sure why struct is "awful".  At this point, its not a question
> of whether I like it or not.  I just need a way to get the first bytes
> of the file read in as POD.  I actually did make Sheader into a class,
> but that really makes little difference.  The following doesn't appear
> to get the correct data into the class members:
>
> #include <iostream>
>
> class Header {
> public:
>   Header(){}
>   Header(std::istream& in);
>   std::ostream& print(std::ostream& out);
> private:
>   long  attr_start;
>   long  form_start;
>   short fsize;
>   short attr_size;
>   short lastfieldidx;
>   short num_inputs;
>   char  oldheader[200];
>
> };
>
> Header::Header(std::istream& in) {
>   in>>attr_start
>     >>form_start
>     >>fsize
>     >>attr_size
>     >>lastfieldidx
>     >>num_inputs;
>
> }
>
> std::ostream& Header::print(std::ostream& out) {
>
>   return out
>     <<"attr_start   = " << (long)attr_start   << std::endl
>     <<"form_start   = " << (long)form_start   << std::endl
>     <<"fsize        = " << (short)fsize        << std::endl
>     <<"attr_size    = " << (short)attr_size    << std::endl
>     <<"lastfieldidx = " << (short)lastfieldidx << std::endl
>     <<"num_inputs   = " << (short)num_inputs   << std::endl;
>
>     //oldheader[200];
>
> }
>
> #include <fstream>
> #include <sstream>
>
> int main(int argc, char* argv[]) {
>   if (argc < 1) { std::cerr<<"enter a file name"<<std::endl; return
> -1; }
>
>   std::ifstream inf(argv[1]);
>   if(inf) {
>     Header header(inf);
>     header.print(std::cout);
>     return 0;
>   }
>
>
>
>
>
>
>
> }
This appears to work, but it sure is brutal:

Header::Header(std::istream& in) {
  in.read( reinterpret_cast<char*>(this), sizeof(*this) );
}

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

#7858 — Re: How to fread ((void *)header, sizeof (struct Sheader), 1, file); in C++

On 07/16/11 12:23 PM, Hetware wrote:
> On Jul 15, 7:30 pm, Ian Collins<ian-n...@hotmail.com>  wrote:
>> On 07/16/11 10:36 AM, Hetware wrote:
>
>>> I have some ancient c code that reads files stored in raw binary
>>> format.  The first 216 bytes are to be read into an instance of:
>>
>>> struct Sheader
>>> {
>>>     long    attr_start;
>>>     long    form_start;
>>>     short   fsize;
>>>     short   attr_size;
>>>     short   lastfieldidx;
>>>     short   num_inputs;
>>>     char    oldheader[200];
>>> };
>>
>>> The way the C code does this is by fread ((void *)header, sizeof
>>> (struct Sheader), 1, file);  Is there a better way to do this in C++?
>>
>> Not really (but you don't need 'struct" in there), it's awful in C and
>> it will still be awful in C++!
>
> I'm not sure why struct is "awful".

Storing raw binary is awful!

> At this point, its not a question
> of whether I like it or not.  I just need a way to get the first bytes
> of the file read in as POD.  I actually did make Sheader into a class,
> but that really makes little difference.  The following doesn't appear
> to get the correct data into the class members:
>
> #include<iostream>
>
> class Header {
> public:
>    Header(){}
>    Header(std::istream&  in);
>    std::ostream&  print(std::ostream&  out);
> private:
>    long	attr_start;
>    long	form_start;
>    short	fsize;
>    short	attr_size;
>    short	lastfieldidx;
>    short num_inputs;
>    char	oldheader[200];
>
> };
>
>
> Header::Header(std::istream&  in) {
>    in>>attr_start
>      >>form_start
>      >>fsize
>      >>attr_size
>      >>lastfieldidx
>      >>num_inputs;
> }

You are using formatted input operators to read raw data (which is one 
reason raw data is evil).

If you have to read in the raw binary data, you have to use in.read(), 
which will look pretty much the same as the C code.

-- 
Ian Collins

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

#7859

On Jul 15, 9:18 pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 07/16/11 12:23 PM, Hetware wrote:
>
>
>
>
>
>
>
>
>
> > On Jul 15, 7:30 pm, Ian Collins<ian-n...@hotmail.com>  wrote:
> >> On 07/16/11 10:36 AM, Hetware wrote:
>
> >>> I have some ancient c code that reads files stored in raw binary
> >>> format.  The first 216 bytes are to be read into an instance of:
>
> >>> struct Sheader
> >>> {
> >>>     long    attr_start;
> >>>     long    form_start;
> >>>     short   fsize;
> >>>     short   attr_size;
> >>>     short   lastfieldidx;
> >>>     short   num_inputs;
> >>>     char    oldheader[200];
> >>> };
>
> >>> The way the C code does this is by fread ((void *)header, sizeof
> >>> (struct Sheader), 1, file);  Is there a better way to do this in C++?
>
> >> Not really (but you don't need 'struct" in there), it's awful in C and
> >> it will still be awful in C++!
>
> > I'm not sure why struct is "awful".
>
> Storing raw binary is awful!
>
>
>
>
>
>
>
>
>
> > At this point, its not a question
> > of whether I like it or not.  I just need a way to get the first bytes
> > of the file read in as POD.  I actually did make Sheader into a class,
> > but that really makes little difference.  The following doesn't appear
> > to get the correct data into the class members:
>
> > #include<iostream>
>
> > class Header {
> > public:
> >    Header(){}
> >    Header(std::istream&  in);
> >    std::ostream&  print(std::ostream&  out);
> > private:
> >    long    attr_start;
> >    long    form_start;
> >    short   fsize;
> >    short   attr_size;
> >    short   lastfieldidx;
> >    short num_inputs;
> >    char    oldheader[200];
>
> > };
>
> > Header::Header(std::istream&  in) {
> >    in>>attr_start
> >      >>form_start
> >      >>fsize
> >      >>attr_size
> >      >>lastfieldidx
> >      >>num_inputs;
> > }
>
> You are using formatted input operators to read raw data (which is one
> reason raw data is evil).
>
> If you have to read in the raw binary data, you have to use in.read(),
> which will look pretty much the same as the C code.
>
> --
> Ian Collins

This code was delivered in 1993.  Some of it is over 20 years old.  I
don't have much say in how it is implemented.  No one seems to know
how it works.  That's why I'm trying to pick it apart.  It's still in
production, so despite all of it's warts, it has done the job it was
intended for.

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

#8613

On Jul 16, 2:18 am, Ian Collins <ian-n...@hotmail.com> wrote:

> You are using formatted input operators to read raw data (which is one
> reason raw data is evil).

I now have a vision of Ian's computer in which his video, music and
photographs are stored as Base64 encoded XML ;)

There's nothing wrong with raw binary data as long as you:
i) specify the widths and endianness.
ii) Don't write/read whole structures where you can't be absolutely
100% sure the padding won't change... and probably not even then.

Next time you're working on an SoC with 32k of non-volatile storage,
you'll come round to binary data.

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

#7860

On Jul 16, 5:23 am, Hetware <fjols...@gmail.com> wrote:
> On Jul 15, 7:30 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
>
>
>
>
>
>
>
>
> > On 07/16/11 10:36 AM, Hetware wrote:
> > > I have some ancient c code that reads files stored in raw binary
> > > format.  The first 216 bytes are to be read into an instance of:
>
> > > struct Sheader
> > > {
> > >    long    attr_start;
> > >    long    form_start;
> > >    short   fsize;
> > >    short   attr_size;
> > >    short   lastfieldidx;
> > >    short   num_inputs;
> > >    char    oldheader[200];
> > > };
>
> > > The way the C code does this is by fread ((void *)header, sizeof
> > > (struct Sheader), 1, file);  Is there a better way to do this in C++?
>
> > Not really (but you don't need 'struct" in there), it's awful in C and
> > it will still be awful in C++!
>
> > --
> > Ian Collins
>
> I'm not sure why struct is "awful".  At this point, its not a question
> of whether I like it or not.  I just need a way to get the first bytes
> of the file read in as POD.  I actually did make Sheader into a class,
> but that really makes little difference.  The following doesn't appear
> to get the correct data into the class members:
>
> #include <iostream>
>
> class Header {
> public:
>   Header(){}
>   Header(std::istream& in);
>   std::ostream& print(std::ostream& out);
> private:
>   long  attr_start;
>   long  form_start;
>   short fsize;
>   short attr_size;
>   short lastfieldidx;
>   short num_inputs;
>   char  oldheader[200];
>
> };
>
> Header::Header(std::istream& in) {
>   in>>attr_start
>     >>form_start
>     >>fsize
>     >>attr_size
>     >>lastfieldidx
>     >>num_inputs;
>
> }
>
> std::ostream& Header::print(std::ostream& out) {
>
>   return out
>     <<"attr_start   = " << (long)attr_start   << std::endl
>     <<"form_start   = " << (long)form_start   << std::endl
>     <<"fsize        = " << (short)fsize        << std::endl
>     <<"attr_size    = " << (short)attr_size    << std::endl
>     <<"lastfieldidx = " << (short)lastfieldidx << std::endl
>     <<"num_inputs   = " << (short)num_inputs   << std::endl;
>
>     //oldheader[200];
>
> }
>
> #include <fstream>
> #include <sstream>
>
> int main(int argc, char* argv[]) {
>   if (argc < 1) { std::cerr<<"enter a file name"<<std::endl; return
> -1; }
>
>   std::ifstream inf(argv[1]);
>   if(inf) {
>     Header header(inf);
>     header.print(std::cout);
>     return 0;
>   }
>
>
>
>
>
>
>
> }

First of all you need to open file in binary mode (ios::binary) and
use read function inplace of extraction operator (>>). Then it will
work as it was working in your old 'C' code.

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

#7872

On Fri, 15 Jul 2011 17:23:18 -0700, Hetware wrote:

> I'm not sure why struct is "awful".

Because the size, alignment, and representation (byte-order,
ones-complement vs twos-complement vs sign-bit, etc) of fundamental types
are implementation-dependent, as are the rules which determine the layout
of a structure.

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

#7874

On Fri, 15 Jul 2011 17:23:18 -0700, Hetware wrote:

> I'm not sure why struct is "awful".

Structs are not, and never were, "awful". Some of the ways programmers 
abuse/misuse them are.

Todd

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

#7878

On Jul 16, 1:23 am, Hetware <fjols...@gmail.com> wrote:
> I just need a way to get the first bytes
> of the file read in as POD.  I actually did make Sheader into a class,
> but that really makes little difference.  The following doesn't appear
> to get the correct data into the class members:
>
> #include <iostream>
>
> class Header {
> public:
>   Header(){}
>   Header(std::istream& in);
>   std::ostream& print(std::ostream& out);
> private:
>   long  attr_start;
>   long  form_start;
>   short fsize;
>   short attr_size;
>   short lastfieldidx;
>   short num_inputs;
>   char  oldheader[200];
>
> };

As others have said, it's a bit risky reading a struct in one go as a
chuck of binary. But I think you're asking for trouble if, as you do
above, you then change what the struct is. I'd be inclined to leave
the struct exacty how it was, and if you want it to do more things
then create a class to do these things that has the original struct as
a member.

The reliable thing to do would instead be to read in the data as a
succession of values and do the right thing with each one in turn, but
this is probably unjustified.

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

#7880

Paul N <gw7rib@aol.com> wrote in news:5b0d8614-70e2-4834-aef8-651a79db39c4
@gc3g2000vbb.googlegroups.com:
> As others have said, it's a bit risky reading a struct in one go as a
> chuck of binary. But I think you're asking for trouble if, as you do
> above, you then change what the struct is. I'd be inclined to leave
> the struct exacty how it was, and if you want it to do more things
> then create a class to do these things that has the original struct as
> a member.
> 
> The reliable thing to do would instead be to read in the data as a
> succession of values and do the right thing with each one in turn, but
> this is probably unjustified.

The right and reliable thing to do is unjustified? Would be funny, except 
that I'm afraid this is exactly what many people out there might think.

Cheers
Paavo

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

#7864

Hetware <fjolsvit@gmail.com> wrote in news:b6cebe9c-fb96-43b7-836e-
95b85ca6f03b@q17g2000vby.googlegroups.com:

> I have some ancient c code that reads files stored in raw binary
> format.  The first 216 bytes are to be read into an instance of:
> 
> struct Sheader
> {
>      long     attr_start;
>      long     form_start;
>      short     fsize;
>      short     attr_size;
>      short     lastfieldidx;
>      short      num_inputs;
>      char     oldheader[200];
> };
> 
> The way the C code does this is by fread ((void *)header, sizeof
> (struct Sheader), 1, file);  Is there a better way to do this in C++?

There is nothing wrong with fread() itself. However, reading directly 
into a struct is not so good idea, this may easily break down with the 
next compiler version or platform change. First, you should document the 
binary file format, e.g.

attr_start: offset 0: a 32-bit signed int in little-endian Intel 
representation.
form_start: offset 4: a 32-bit signed int in little-endian Intel 
representation.
fsize: offset 8: a 16-bit signed int in little-endian Intel 
representation.

etc

Then you can implement a proper reading function for that format.

std::vector<char> buffer(216);
// open file in binary mode, read it by fread() or equivalent into the 
buffer, then do:

Sheader h;
h.attr_start = ReadLittleEndianSignedInt32(&buffer[0]);
h.form_start = ReadLittleEndianSignedInt32(&buffer[3]);

etc. This is tedious of course, but protects against datatype 
representation changes (hint-hint: "long" is not 32 bits in quite a lot 
of implementations nowadays) and also against changes in the internal 
padding inside the struct. 

Here ReadLittleEndianSignedInt32() would do the following tasks:

1. on big-endian machines swaps the bytes as needed in a char buffer 
(cannot use the result int32 variable for that as storing a random byte 
sequence there might theoretically yield a trapping representation).

2. interpretes the char buffer as a 4-byte signed integer and returns the 
value. A reinterpret_cast<> cannot be used directly because of alignment 
problems, I think the only standard way is to use memcpy() to copy the 
bytes into an int32_t variable. 

A way to postpone fixing the code would be to keep the current raw struct 
reading and add a bunch of static asserts to ensure the compilation will 
fail when the struct hack breaks down.

#ifdef __BIG_ENDIAN__
#error Not supported!
#endif
BOOST_STATIC_ASSERT(sizeof(long)==4);
BOOST_STATIC_ASSERT(sizeof(short)==2);
BOOST_STATIC_ASSERT(sizeof(Sheader)==216);

hth
Paavo

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

#7922

Paavo Helde <myfirstname@osa.pri.ee> wrote:
> There is nothing wrong with fread() itself. However, reading directly 
> into a struct is not so good idea, this may easily break down with the 
> next compiler version or platform change.

  It's unlikely that a simple compiler version change will change the
geometry of a struct (because it would most probably break many things
even in the standard C library). However, it's true that a change in
platform (eg. 32-bit vs 64-bit, or eg. Intel vs. UltraSparc) will probably
break it.

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

#7926

On 17 Jul 2011 13:46:50 GMT, Juha Nieminen <nospam@thanks.invalid>
wrote:

>Paavo Helde <myfirstname@osa.pri.ee> wrote:
>> There is nothing wrong with fread() itself. However, reading directly 
>> into a struct is not so good idea, this may easily break down with the 
>> next compiler version or platform change.
>
>  It's unlikely that a simple compiler version change will change the
>geometry of a struct (because it would most probably break many things
>even in the standard C library). However, it's true that a change in
>platform (eg. 32-bit vs 64-bit, or eg. Intel vs. UltraSparc) will probably
>break it.


It's not that common, but it has happened.

Plenty of compilers have command line options that can cause such
changes.  A common one controls alignment.  Several compilers have
changed the default alignment for some types over time (from 4 to 8
for doubles happened on several implementations, IIRC, Symantec's C
compiler for the Mac changed the default alignment of chars from 1 to
2 at one point).

As mentioned in another thread, C++ bool has changed over time on
several compilers.  Between MSVC4.2 and MSVC5.0, and between GCC2 and
GCC3 on Alpha, for example (admittedly both of those were a long time
ago).

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

#7868

On 16 heinä, 01:36, Hetware <fjols...@gmail.com> wrote:
> The way the C code does this is by fread ((void *)header, sizeof
> (struct Sheader), 1, file);  Is there a better way to do this in C++?

No. But it could be possible to convert the old data to more C++
readable form. That way you could crush two bugs at the same time:
dump old binary data and make better, more portable loader for
the data.

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

#7927

On Jul 16, 1:36 am, Hetware <fjols...@gmail.com> wrote:
> I have some ancient c code that reads files stored in raw binary
> format.  The first 216 bytes are to be read into an instance of:
>
> struct Sheader
> {
>         long    attr_start;
>         long    form_start;
>         short   fsize;
>         short   attr_size;
>         short   lastfieldidx;
>         short   num_inputs;
>         char    oldheader[200];
>
> };
>
> The way the C code does this is by fread ((void *)header, sizeof
> (struct Sheader), 1, file);  Is there a better way to do this in C++?

You have to parse the data in the file into the struct; for that you
have many alternatives. Reading from the file byte-by-byte is rubbish
and not recommended. Allocate the buffer which is size of the header:

char* buffer = new char[216];

Then fill the buffer with block read. fread works just fine. Or, if
you have a framework to use which has memory mapped I/O, even better:
just lock the region of the file (the first 216 bytes?) and use the
acquired pointer to the mapped region.

Either way, you will end up having a pointer:

char* buffer; // points to the 216 bytes of header data

Then you parse the data. It's useful to create few helper functions
for the parsing, example:

long read_long(char*& p)
{
    long value = p[0] | (p[1] << 8) | (p[2] << 16) | p[3] << 24);
    p += 4;
    return value;
}

Something like that. If you encapsulate the parser into a class, you
don't need to pass the pointer around (you will still be passing
instance of the parser; "this" pointer around but that's another
story).

Also, depending on the endianess of the file you might want to modify
the byte-order in above read_long function (/method).

That's the main gist of it pretty much. I use parser class like that
in all the code I wrote when dealing with binary objects. The
constructor takes pointer to the data and size (for overflow checking
purposes and such) and then just parse away! Main point is to just
dead with the data as region of memory, this way works nicely even
with game consoles like XBOX and PS3, old and new operating systems,
etc. I advocate this as the "best" approach, forget diddling about
with the C++ streams - sure feel free to use them to initialize the
pointer for the parser but it's not really the critical detail.

This is simple, efficient, portable .. there really is no downside. If
you however, read from file, parse, read from file, parse and repeat..
the code isn't any simpler to write but the performance will suck
eventually. If you pick up bad habits, you'll stick with them as they
grow into you - that's bad.

This isn't premature optimization in case someone suggests that. This
is just good practice. Have a nice day.

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

#8387

On Sunday, July 17, 2011 2:32:10 PM UTC-4, hanukas wrote:

> You have to parse the data in the file into the struct; for that you
> have many alternatives. Reading from the file byte-by-byte is rubbish
> and not recommended. Allocate the buffer which is size of the header:
> 
> char* buffer = new char[216];
> 
> Then fill the buffer with block read. fread works just fine. Or, if
> you have a framework to use which has memory mapped I/O, even better:
> just lock the region of the file (the first 216 bytes?) and use the
> acquired pointer to the mapped region.
> 
> Either way, you will end up having a pointer:
> 
> char* buffer; // points to the 216 bytes of header data
> 
> Then you parse the data. It's useful to create few helper functions
> for the parsing, example:
> 
> long read_long(char*& p)
> {
>     long value = p[0] | (p[1] << 8) | (p[2] << 16) | p[3] << 24);
>     p += 4;
>     return value;
> }
> 
> Something like that. If you encapsulate the parser into a class, you
> don't need to pass the pointer around (you will still be passing
> instance of the parser; "this" pointer around but that's another
> story).
> 
> Also, depending on the endianess of the file you might want to modify
> the byte-order in above read_long function (/method).
> 
> That's the main gist of it pretty much. I use parser class like that
> in all the code I wrote when dealing with binary objects. The
> constructor takes pointer to the data and size (for overflow checking
> purposes and such) and then just parse away! Main point is to just
> dead with the data as region of memory, this way works nicely even
> with game consoles like XBOX and PS3, old and new operating systems,
> etc. I advocate this as the "best" approach, forget diddling about
> with the C++ streams - sure feel free to use them to initialize the
> pointer for the parser but it's not really the critical detail.
> 
> This is simple, efficient, portable .. there really is no downside. If
> you however, read from file, parse, read from file, parse and repeat..
> the code isn't any simpler to write but the performance will suck
> eventually. If you pick up bad habits, you'll stick with them as they
> grow into you - that's bad.
> 
> This isn't premature optimization in case someone suggests that. This
> is just good practice. Have a nice day.

Your code still appears to depend on the specifics of the underlying architecture and compiler configuration.  Since I can rely on the binary structure of the files as long as the platform remains the same, it's safe to  in.read(reinterpret_cast<char*>(&h), sizeof(h));  I don't particularly like that approach, but it works for what I'm doing.  I create struct instances to hold the data as it's read in.  I then pass the individual data members as parameters to a corresponding class structure.  

It's cumbersome and ugly, but gives me the advantage that the new data classes make no assumptions about the underlying data representation.

E.g., Header is a class corresponding to Sheader h struct.

std::ifstream in(_fileName.c_str(), std::ios::binary);
  if(in) {
    Sheader h;
    in.read(reinterpret_cast<char*>(&h), sizeof(h));
    _header=Header(h.attr_start  
		   ,h.form_start  
		   ,h.fsize	     
		   ,h.attr_size   
		   ,h.lastfieldidx
		   ,h.num_inputs  
		   ,std::string(h.oldheader, 200)
		   );

This code lives in the constructor of a Form class which has a Header member object.  This isolates the ugliness to one function.  I will need a corresponding binary serialization function to save the Form in the current format.

I will likely create an XML representation of the Form class.  Unfortunately this will likely remain mostly an academic exercise.  My immediate goal is to understand the semantics of the underlying data.   

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

#8394

On Mon, 2011-08-01, Hetware wrote:
> On Sunday, July 17, 2011 2:32:10 PM UTC-4, hanukas wrote:
>
>> You have to parse the data in the file into the struct; for that you
>> have many alternatives. Reading from the file byte-by-byte is rubbish
>> and not recommended. Allocate the buffer which is size of the header:
>> 
>> char* buffer = new char[216];
>> 
>> Then fill the buffer with block read. fread works just fine. Or, if
>> you have a framework to use which has memory mapped I/O, even better:
>> just lock the region of the file (the first 216 bytes?) and use the
>> acquired pointer to the mapped region.
>> 
>> Either way, you will end up having a pointer:
>> 
>> char* buffer; // points to the 216 bytes of header data
>> 
>> Then you parse the data. It's useful to create few helper functions
>> for the parsing, example:
>> 
>> long read_long(char*& p)
>> {
>>     long value = p[0] | (p[1] << 8) | (p[2] << 16) | p[3] << 24);
>>     p += 4;
>>     return value;
>> }
>> 
>> Something like that. If you encapsulate the parser into a class, you
>> don't need to pass the pointer around (you will still be passing
>> instance of the parser; "this" pointer around but that's another
>> story).
>> 
>> Also, depending on the endianess of the file you might want to modify
>> the byte-order in above read_long function (/method).
>> 
>> That's the main gist of it pretty much. I use parser class like that
>> in all the code I wrote when dealing with binary objects. The
>> constructor takes pointer to the data and size (for overflow checking
>> purposes and such) and then just parse away! Main point is to just
>> dead with the data as region of memory, this way works nicely even
>> with game consoles like XBOX and PS3, old and new operating systems,
>> etc. I advocate this as the "best" approach, forget diddling about
>> with the C++ streams - sure feel free to use them to initialize the
>> pointer for the parser but it's not really the critical detail.
>> 
>> This is simple, efficient, portable .. there really is no downside. If
>> you however, read from file, parse, read from file, parse and repeat..
>> the code isn't any simpler to write but the performance will suck
>> eventually. If you pick up bad habits, you'll stick with them as they
>> grow into you - that's bad.
>> 
>> This isn't premature optimization in case someone suggests that. This
>> is just good practice. Have a nice day.

> Your code still appears to depend on the specifics of the underlying
> architecture and compiler configuration.  [...]

How, exactly?  Replace his long with uint32_t (or unsigned int plus an
assertion that it's at least 32 bits) and his posting would describe
my own code, which I believe is fairly portable.

/Jorgen

PS. Please wrap your lines. Some of them were ~500 characters wide.

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

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

#8552

On Jul 17, 7:32 pm, hanukas <ju...@liimatta.org> wrote:
> On Jul 16, 1:36 am, Hetware <fjols...@gmail.com> wrote:
> > I have some ancient c code that reads files stored in raw binary
> > format.  The first 216 bytes are to be read into an instance of:

> > struct Sheader
> > {
> >         long    attr_start;
> >         long    form_start;
> >         short   fsize;
> >         short   attr_size;
> >         short   lastfieldidx;
> >         short   num_inputs;
> >         char    oldheader[200];
> > };

> > The way the C code does this is by fread ((void *)header, sizeof
> > (struct Sheader), 1, file);  Is there a better way to do this in C++?

Of course, that doesn't even work portably in C.

> You have to parse the data in the file into the struct; for that you
> have many alternatives. Reading from the file byte-by-byte is rubbish
> and not recommended.

Why?  std::istream has been designed so that things like
std::istream::get() can easily be inlined (since they don't call
a virtual function if there is data in the buffer), so the
overhead shouldn't be that bad.  Other than that, it's really a
style issue as to whether you prefer to use std::istream::get or
play with pointers or iterators.  (Having said that, I
personally prefer the pointer solution as well.  Old habits,
etc.)

> Allocate the buffer which is size of the header:

> char* buffer = new char[216];

> Then fill the buffer with block read. fread works just fine.

As does std::istream::read.  In both cases, don't forget to open
the stream in binary mode.

> Or, if
> you have a framework to use which has memory mapped I/O, even better:
> just lock the region of the file (the first 216 bytes?) and use the
> acquired pointer to the mapped region.

> Either way, you will end up having a pointer:

> char* buffer; // points to the 216 bytes of header data

> Then you parse the data. It's useful to create few helper functions
> for the parsing, example:

> long read_long(char*& p)
> {
>     long value = p[0] | (p[1] << 8) | (p[2] << 16) | p[3] << 24);
>     p += 4;
>     return value;
> }

Just a nit, but I tend to convert the pointer to unsigned char*
(or unsigned char const* when reading).  The use of an operator
(<<) provokes integral promotion, and if char is signed (as it
usually is by default), you're likely to get some bits set that
you didn't expect.

More generally, I've usually seen the convention that plain char
is only used for characters; small integers are signed char, and
raw memory is unsigned char.

> Something like that. If you encapsulate the parser into a class, you
> don't need to pass the pointer around (you will still be passing
> instance of the parser; "this" pointer around but that's another
> story).

> Also, depending on the endianess of the file you might want to modify
> the byte-order in above read_long function (/method).

> That's the main gist of it pretty much. I use parser class like that
> in all the code I wrote when dealing with binary objects. The
> constructor takes pointer to the data and size (for overflow checking
> purposes and such) and then just parse away! Main point is to just
> dead with the data as region of memory, this way works nicely even
> with game consoles like XBOX and PS3, old and new operating systems,
> etc. I advocate this as the "best" approach, forget diddling about
> with the C++ streams - sure feel free to use them to initialize the
> pointer for the parser but it's not really the critical detail.

Another variant I've seen used is to access the streambuf
directly.  For raw data, that's often as convenient as going
through istream.

--
James Kanze

[toc] | [prev] | [standalone]

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

csiph-web