-1

Code =>

#include<stdio.h>

typedef struct {
    unsigned short c2;
    unsigned long c4;
} TAKECH;

int main() {
    TAKECH tch;
    FILE *fp_in;

    fp_in = fopen("in.txt","rb");

    fread(&tch,6,1,fp_in);

    printf("First two bytes: %x\n",tch.c2);
    printf("Next four bytes: %x\n",tch.c4);

    fclose(fp_in);

    return 0;
}

Output =>

First two bytes: 6261
Next four bytes: bfd56665

in.txt =>

abcdef

Hexeditor(vim editor :%!xxd) show this =>

0000000: 6162 6364 6566 0a                        abcdef.

Need explanation of output:

First two bytes: 6261 <-- Why is it in reverse order?

First two bytes: 6162 <-- Shouldn't this be?

Why can't I get 6364 in output? How can i get the next four bytes(6364 6566) with printf("Next four bytes: %x\n",tch.c4); Why do I get Next four bytes: bfd56665 , where does bfd5 come from?

Any answer will be highly appreciated.

Thanks in advance.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56
cola
  • 12,198
  • 36
  • 105
  • 165
  • 3
    Why is this question tagged with c++? – Biruk Abebe May 23 '16 at 07:42
  • What do you expect `fread(&tch,6,1,fp_in);` to do? Honestly, the simplest answer is that doing that really doesn't make very much sense. – David Schwartz May 23 '16 at 08:55
  • @DavidSchwartz, I want to assign first two bytes to tch.c2(ab) and last four bytes to tch.c4(cdef) . – cola May 23 '16 at 09:02
  • @shibly So then why don't you write code to do that?! Create a six-byte buffer, like `char buf[6];`, read the file into that, and then put each byte exactly where you want it, like `tch.c2=buf[0]; tch.c2<<=8; tch.c2|=buf[1];` or whatever you want. You can't randomly cast pointers and expect sensible results, you have to code what you actually want. (Also, you asked the wrong question. State what you want to do and ask for a good way to do it. Don't ask for help solving a problem in a way that doesn't make sense in the first place!) – David Schwartz May 23 '16 at 09:05
  • @DavidSchwartz, Is it possible to do the same using structure instead of buffer? – cola May 23 '16 at 09:19
  • 1
    @shibly Is it possible, yes. Is it smart, no. Why write complex, gratuitously non-portable code when there's a much simpler, 100% portable solution? And, as a bonus, *you're actually writing code that does precisely what it is you want to do* rather than using a combination of circumstances that you carefully rig to happen to do what you want, and then your code breaks if those circumstances change. – David Schwartz May 23 '16 at 09:38

6 Answers6

3

You should use a buffer for fread (see the example on http://en.cppreference.com/w/c/io/fread), not a struct.

Because of padding you're getting just two "correct" bytes from the file (65 and 66). The other bytes of c4 are uninitialized.

For the order "issue" you can take a look at: Why does fread mess with my byte order?


This is machine/compiler-dependent so actual results may vary.

typedef struct
{
  uint16_t c2;
  uint32_t c4;
} TAKECH;

sizeof(TAKECH) is 8 (not 6 = sizeof(c2) + sizeof(c4)): padding is added to satisfy alignment constraints (data structure alignment impacts both performance and correctness of programs).

typedef struct
{
  uint16_t c2;  /* 2 bytes */
                /* 2 padding bytes */
  uint32_t c4;  /* 4 bytes */
} TAKECH;

(see also Why isn't sizeof for a struct equal to the sum of sizeof of each member?).

Community
  • 1
  • 1
manlio
  • 18,345
  • 14
  • 76
  • 126
  • 1
    I would also mention that the OP is assuming that `short` is 2 bytes wide and `long` is 4 bytes wide which is wrong. `short` is *at least* 2 bytes wide and `long` is *at least* 4 bytes wide. – Mohamad Elghawi May 23 '16 at 07:55
  • @manlio, I could not understand the concept of padding, can you elaborate it more? How can i get the value `6364` ? – cola May 23 '16 at 08:18
  • Ah, I understood padding from here: http://stackoverflow.com/questions/6968468/padding-in-structures-in-c – cola May 23 '16 at 08:26
  • @shibly I've added some details. – manlio May 23 '16 at 08:45
  • @manlio, I understood the concept of padding, How can i get the value `6364` – cola May 23 '16 at 09:32
  • @shibly You could use two calls to fread: `fread(&tch.c2, sizeof(tch.c2), 1, fp_in);` and `fread(&tch.c4, sizeof(tch.c4), 1, fp_in);` (but you should adjust for endianness). – manlio May 23 '16 at 10:23
2

First two bytes: 6261 <-- Why is it in reverse order?

Clearly you are running this code on a little-endian CPU architecture. Your issue relates to how bytes are ordered in memory.

Here's an explanation.

trojanfoe
  • 120,358
  • 21
  • 212
  • 242
1

Hi might I suggest you clear your tch struct before assignment as it is filled with garbage.

Yeah I really don't understand why I got down voted here but you know I will add some code to prove my point:

mmcmbp:scratch abe$ cat main.c 

#include <stdio.h>
#include <string.h>

typedef struct {
    unsigned short c2;
    unsigned long c4;
} TAKECH;

int main() {
    TAKECH tch;
    FILE *fp_in;

    memset(&tch, 0, sizeof(TAKECH));

    printf("Before\n");
    printf("First two bytes: %hu\n",tch.c2);
    printf("Next four bytes: %lu\n",tch.c4);

    fp_in = fopen("in.txt","rb");

    fread(&tch,6,1,fp_in);

    printf("After:\n");
    printf("First two bytes: %hu\n",tch.c2);
    printf("Next four bytes: %lu\n",tch.c4);

    fclose(fp_in);

    return 0;
}

Compilation:

mmcmbp:scratch abe$ clang main.c -o main

Execution:

mmcmbp:scratch abe$ ./main

Before
First two bytes: 0
Next four bytes: 0
After:
First two bytes: 25185
Next four bytes: 0

And as per the ordering of the bytes, yes the endianness can dictate it and is what others have stated.

Abraham
  • 230
  • 3
  • 15
1

Most compilers support the "pack" pragma, which allows you to specify how struct members are laid out in memory. This example shows that packing with size-1 member alignment will make your struct match the file's layout. You don't want to use this kind of packing when you don't need it however, because it slows down performance and might cause other probblems because of misaligned memory accesses.

#include <iostream>
#include <cstring>

typedef struct {
    unsigned short c2;
    unsigned long c4;
} TAKECH;

#pragma pack(push,1)
typedef struct {
    unsigned short c2;
    unsigned long c4;
} TAKECH_packed_1;
#pragma pack(pop)

const unsigned char data[] = "\x61\x62\x63\x64\x65\x66\x0a\xff\xfe\xfd\xfc";

int main() {
    TAKECH original;
    std::memcpy(&original, &data, sizeof(original));
    std::cout << std::hex;
    std::cout << "Default packing:\n";
    std::cout << "    c2: " << original.c2 << '\n';
    std::cout << "    c4: " << original.c4 << '\n';

    TAKECH_packed_1 packed;
    std::memcpy(&packed, &data, sizeof(packed));
    std::cout << "\nByte packing:\n";
    std::cout << "    c2: " << packed.c2 << '\n';
    std::cout << "    c4: " << packed.c4 << '\n';
}

This outputs

Default packing:
    c2: 6261
    c4: ff0a6665

Byte packing:
    c2: 6261
    c4: 66656463
Christopher Oicles
  • 3,017
  • 16
  • 11
0

Your code would work if TAKECH were laid out like this:

    Low address                                 High address
    |        c2       |                  c4                |
    | Byte 1 | Byte 0 | Byte 3 |  Byte 2 | Byte 1 | Byte 0 |

but it's actually laid out like this:

    Low address                                              High address
    |        c2       |   Padding   |                  c4               |
    | Byte 0 | Byte 1 |      |      | Byte 0 | Byte 1 | Byte 2 | Byte 3 |

tch:    61       62      63     64      65       66    junk(d5) junk(bf)

The first issue, the ordering, is due to your computer being little-endian - the least significant byte of a multi-byte integer is stored at the lower address.

The second issue is due to your assumption that sizeof(TAKECH) is six.
It's not; it has been padded to make the address of c4 a multiple of sizeof(unsigned long).
This leads to part of tch (the "top" two bytes of tch.c4) being uninitialised when you only read six bytes.

A reliable and portable solution is to read each member separately,

fread(&tch.c2, sizeof(tch.c2), 1, fp_in);
fread(&tch.c4, sizeof(tch.c4), 1, fp_in);

and adjust the endianness afterwards.

Summary:

  • Always use sizeof instead of relying on assumptions.
  • When dealing with binary data, you must be aware of both padding and endianness.
molbdnilo
  • 64,751
  • 3
  • 43
  • 82
0

Changing a TAKECH struct as follow :

typedef struct {
    unsigned short c2;
    unsigned long c4;
} __attribute__((packed)) TAKECH;

Here's an explanation about __attribute__((packed).

The byte order is up to a little-endian cpu or a big-endian cpu. If you executed your code in a big-endian cpu, your opinion was right. But The PC is a little-endian cpu. Currently the most platform used a little-endian mode, although supported a big-endian mode. Here's more detail.

Community
  • 1
  • 1
PeanutStars
  • 18
  • 1
  • 5