current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have a file with an specific format. It's divided in stripes of 1058816 bytes:

  • 4096 bytes with only a 64 bit unsigned integer.
  • An array of 512 64 bit unsigned integer.
  • An array of 512 32 bit unsigned integer.
  • An array of 512 elements with 2048 bytes each one.

I have to read the file, change some values and write it again.

This is what I'm doing to work with it:

struct Segment {
        char bytes[2048];
};

struct Metadata {
        uint64_t serial;
};

class Stripe {
        uint8_t ptr[STRIPE_SIZE];

        Metadata *metadata;
        uint64_t *hash_array;
        uint32_t *size_array;
        Segment *segment_array;

        do_something(fstream_obj)
        {
                fstream_obj.read(ptr, STRIPE_SIZE);

                metadata = new(ptr) COSSMetadata;
                hash_array = new(ptr + METADATA_BYTES) uint64_t[ARRAY_SIZE];
                size_array = new(ptr + METADATA_BYTES + HASH_ARRAY_BYTES) uint32_t[ARRAY_SIZE];
                segment_array = new(ptr + METADATA_BYTES + HASH_ARRAY_BYTES + SIZE_ARRAY_BYTES) Segment[ARRAY_SIZE];

                // .... code to change the the values.....

                fstream_obj.write(ptr, STRIPE_SIZE);

        }
};

I have more experience with C. I'm not sure if this is the best to do what I want. I think that is faster, because I'm not doing any conversion and the information in ptr already is in the format of the structures. I want to hear opinios of more experience C++ coderes.

share|improve this question

migrated from stackoverflow.com Jun 17 '12 at 23:47

This question came from our site for professional and enthusiast programmers.
    
@user315052 Sorry, my example is broken, I'll fix it. –  diegows Jun 16 '12 at 2:28
    
And looking on it better, the pointer arithmetic is broken too :) –  diegows Jun 16 '12 at 2:33

3 Answers 3

up vote 4 down vote accepted

I'm not 100% sure how idiomatic this is, but since your data is so structured, it might be worth representing the entire structure as just that: a POD struct that contains the exact sizes and layout desired.

struct Metadata {
    uint64_t serial;
    char _padding_[METADATA_BYTES - 8];
};

struct Segment {
    char bytes[2048];
};

struct Stripe {
    Metadata metadata;
    uint64_t hash_array[512];
    uint32_t size_array[512];
    Segment segment_array[512];
};

(There is probably a better way of doing the padding.)

This means that the layout in memory is exactly the same as the layout on disk, and new Stripe will allocate the appropriate amount of memory with all the components contiguous as desired, without having to do the specific placement yourself. This means you can just allocate the structure, then file.read(stripe, sizeof(Stripe)) and file.write(stripe, sizeof(Stripe)) directly.

You could put a wrapper class around it:

class Stripe {
    Stripe_struct data;

    // methods...
}

Or just convert the original Stripe struct to a class itself.

share|improve this answer
    
I like this solution. I don't know what I did that horrible thing at first :) –  diegows Jun 18 '12 at 18:11
up vote 2 down vote

Placement new is probably the way to go, but you're invoking undefined behaviour because your data buffers may have a different alignment than what you create with placement new and force in there.

By using these sort of members:

class Stripe {
    std::tr1::aligned_storage<sizeof(Metadata),std::tr1::alignment_of<Metadata>::value> metadata_area;
    Metadata* metadata;
  public:
    Stripe()
      : metadata(0) /*...*/
      {
    }
    void postConstructionInit() {
      if (metadata)
        metadata->~Metadata();
      metadata = new(&metadata_area) Metadata();
    }
    ~Stripe() {
      if (metadata)
        metadata->~Metadata();
    }
};

You can get around that problem, while still allowing late initialisation.

Note that if you can manage to write your initialisation code in the constructor, you can make your life easier:

class Stripe {
    Metadata metadata;
  public:
    Stripe()
      : metadata() /* actual initialisation */
      {
    }
    // note that in this case, no explicit destructor implementation is *required*
};
share|improve this answer
up vote 0 down vote

Well, if you are reading and writing to the same file you could just seekp(loc) to the required bit you want to change and write just those bits. Or you could just copy the file and edit the copy as I've described.

To copy quickly:

  std::ifstream fromf(from.string(), std::ios::binary);
  if (fromf.fail())
    {
      ; // Error...
      return;
    }
  std::ofstream tof(to.string(), std::ios::binary);
  if (tof.fail())
    {
      ; // Error...
      return;
    }
  tof << fromf.rdbuf();

Hopefully, you don't have to worry about endianness of the file. If you do that you might have to swap bytes of individual integers.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.