what is wrong with this code?

[Nobody]

Let me start off with a brief overview...

This part is not really important, just saying what its for...

I had been working on a Windows GUI library (DLL) when suddenly my boss told
he wanted a static .lib version so he could give a customer a single .exe
file for the "free version".

Certain parts of my library are required to live inside a DLL.

My solution was to move those parts into a seperate small DLL that contained
only those functions and encode it into the DLL or .lib as a byte array.

Then at runtime write out the DLL to a temporary location and delete it
afterwards...

Yes, dirty I know...

Anyways, I've got it mostly working, but my binary encoder seems to be
adding an extra byte onto the end of the file. Does anyone see a problem
with this code:

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

Also, while you are at it, here is an interesting question... my DLL was
initially 745k in debug mode. I encoded a 40k DLL and ended with obviously a
40k array of bytes. This added ZERO size to my DLL version and only 2k to my
static lib version.

After correctly the extra byte by hand, I have confirmed that all the data
is there because I did a compare on the original DLL and the DLL I wrote out
and they are exactly the same.

And since I am doing no compression on this data... where the hell is the
compiler sticking 40k of data? surely there isn't THAT much padding??

Anyways, here is the code... does any one see why it is writing an extra
byte??

int _tmain(int argc, _TCHAR* argv[])
{
if (argc < 3)
{
printf("\nUsage: BINENC inputfile outputfile\n");
return 0;
}

FILE* fpIn = fopen(argv[1], "rb");

if (!fpIn)
{
printf("\nError: Cannot open input file \"%s\"\n", argv[1]);
return 0;
}

FILE* fpOut = fopen(argv[2], "wt");

if (!fpOut)
{
printf("\nError: Cannot open output file \"%s\"\n", argv[2]);
fclose(fpIn);
return 0;
}

int nByteCount = 0;
int nCount = 0;
unsigned char chBuf;

while (fread(&chBuf, 1, 1, fpIn) != 0)
{
// beginning of line

if (nCount == 0)
fprintf(fpOut, "\"");

// write the formatted hex character

fprintf(fpOut, "\\x%02x", chBuf);

// next position

nCount++;
nByteCount++;

// end of line

if (nCount == 35)
{
fprintf(fpOut, "\"\n");
nCount = 0;
}
}

fprintf(fpOut, "\"\n");

fclose(fpIn);
fclose(fpOut);

printf("\nEncoded %d bytes\n", nByteCount);

return 0;
}

 

[Alf P. Steinbach]

Anyways, I've got it mostly working, but my binary encoder seems to be
adding an extra byte onto the end of the file. Does anyone see a problem
with this code:

Yes, see below for answers both to your immediate problem and some
others.

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

Also, while you are at it, here is an interesting question... my DLL was
initially 745k in debug mode. I encoded a 40k DLL and ended with obviously a
40k array of bytes. This added ZERO size to my DLL version and only 2k to my
static lib version.

After correctly the extra byte by hand, I have confirmed that all the data
is there because I did a compare on the original DLL and the DLL I wrote out
and they are exactly the same.

And since I am doing no compression on this data... where the hell is the
compiler sticking 40k of data? surely there isn't THAT much padding??

The question does not parse.

Anyways, here is the code... does any one see why it is writing an extra
byte??

int _tmain(int argc, _TCHAR* argv[])

_tmain is not standard C++.

TCHAR is not standard C++.

And in particular, your code DOES NOT support the intended usage of
these vendor-specific identifiers, so using them is highly misleading.

{
if (argc < 3)
{
printf("\nUsage: BINENC inputfile outputfile\n");

Print this to standard error.

return 0;
}

Be nice to the user: don't stick in unnecessary newlines, which tend
to fill up the console or console window.

FILE* fpIn = fopen(argv[1], "rb");

if (!fpIn)
{
printf("\nError: Cannot open input file \"%s\"\n", argv[1]);

Print this to standard error.

return 0;
}

Here you should return EXIT_FAILURE, not 0 which is synonymous with
EXIT_SUCCESS.

FILE* fpOut = fopen(argv[2], "wt");

Here you're opening the file in text mode. In this mode some compiler
and/or operating system dependent translation may take place. In
particular, in your case, a Ctrl Z may be appended at the end as an
end-of-file marker, which seems to be your immediate problem.

if (!fpOut)
{
printf("\nError: Cannot open output file \"%s\"\n", argv[2]);

Print this to standard error.

fclose(fpIn);
return 0;

Here you should return EXIT_FAILURE, not 0 which is synonymous with
EXIT_SUCCESS.

}

It's a good idea to use exception handling, then you don't have to
redundantly repeat clean-up operations everywhere an exit is desired.

int nByteCount = 0;
int nCount = 0;
unsigned char chBuf;

while (fread(&chBuf, 1, 1, fpIn) != 0)
{

No error checking.

// beginning of line

if (nCount == 0)
fprintf(fpOut, "\"");

No error checking.

// write the formatted hex character

fprintf(fpOut, "\\x%02x", chBuf);

No error checking.

// next position

nCount++;
nByteCount++;

Preferentially use ++x, not x++.

// end of line

if (nCount == 35)
{
fprintf(fpOut, "\"\n");

No error checking.

Also, you risk ending up with a line at the end that contains a single
quotation mark, i.e. not syntactically correct wrt. your file format.

To be precise this case arises when nCount == 35 after reading the
last character of the input file.

nCount = 0;
}
}

fprintf(fpOut, "\"\n");

No error checking.

fclose(fpIn);
fclose(fpOut);

printf("\nEncoded %d bytes\n", nByteCount);

This spurious output makes it difficult to use the program in a script.

Avoid spurious output à la Microsoft.

 

[Nobody]

Alf,

It doesn't appear to be appending a CTRL-Z to the end... its sticking an
extra "\x00" on the end.

If it was appending a CTRL-Z, surely it wouldnt show up in my formatted
byte array right?

The output appears to be formatted correctly except for a \x00 at the end.

Anyways, I've got it mostly working, but my binary encoder seems to be
adding an extra byte onto the end of the file. Does anyone see a problem
with this code:

Yes, see below for answers both to your immediate problem and some
others.

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

Also, while you are at it, here is an interesting question... my DLL was
initially 745k in debug mode. I encoded a 40k DLL and ended with obviously a
40k array of bytes. This added ZERO size to my DLL version and only 2k to my
static lib version.

After correctly the extra byte by hand, I have confirmed that all the data
is there because I did a compare on the original DLL and the DLL I wrote out
and they are exactly the same.

And since I am doing no compression on this data... where the hell is the
compiler sticking 40k of data? surely there isn't THAT much padding??

The question does not parse.

Anyways, here is the code... does any one see why it is writing an extra
byte??

int _tmain(int argc, _TCHAR* argv[])

_tmain is not standard C++.

TCHAR is not standard C++.

And in particular, your code DOES NOT support the intended usage of
these vendor-specific identifiers, so using them is highly misleading.

{
if (argc < 3)
{
printf("\nUsage: BINENC inputfile outputfile\n");

Print this to standard error.

return 0;
}

Be nice to the user: don't stick in unnecessary newlines, which tend
to fill up the console or console window.

FILE* fpIn = fopen(argv[1], "rb");

if (!fpIn)
{
printf("\nError: Cannot open input file \"%s\"\n", argv[1]);

Print this to standard error.

return 0;
}

Here you should return EXIT_FAILURE, not 0 which is synonymous with
EXIT_SUCCESS.

FILE* fpOut = fopen(argv[2], "wt");

Here you're opening the file in text mode. In this mode some compiler
and/or operating system dependent translation may take place. In
particular, in your case, a Ctrl Z may be appended at the end as an
end-of-file marker, which seems to be your immediate problem.

if (!fpOut)
{
printf("\nError: Cannot open output file \"%s\"\n", argv[2]);

Print this to standard error.

fclose(fpIn);
return 0;

Here you should return EXIT_FAILURE, not 0 which is synonymous with
EXIT_SUCCESS.

}

It's a good idea to use exception handling, then you don't have to
redundantly repeat clean-up operations everywhere an exit is desired.

int nByteCount = 0;
int nCount = 0;
unsigned char chBuf;

while (fread(&chBuf, 1, 1, fpIn) != 0)
{

No error checking.

// beginning of line

if (nCount == 0)
fprintf(fpOut, "\"");

No error checking.

// write the formatted hex character

fprintf(fpOut, "\\x%02x", chBuf);

No error checking.

// next position

nCount++;
nByteCount++;

Preferentially use ++x, not x++.

// end of line

if (nCount == 35)
{
fprintf(fpOut, "\"\n");

No error checking.

Also, you risk ending up with a line at the end that contains a single
quotation mark, i.e. not syntactically correct wrt. your file format.

To be precise this case arises when nCount == 35 after reading the
last character of the input file.

nCount = 0;
}
}

fprintf(fpOut, "\"\n");

No error checking.

fclose(fpIn);
fclose(fpOut);

printf("\nEncoded %d bytes\n", nByteCount);

This spurious output makes it difficult to use the program in a script.

Avoid spurious output à la Microsoft.

return 0;
}

--
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

 

[Chad J McQuinn]

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

So later, when you want to write out the dll that you have encoded, you
do something like

const char buffer[] =
#include "outputdll.h"

where outputdll.h contains the hex-encoded file, contained in double
quotes. Correct?

Well, to answer your question about the extra byte, answer this
question. What is sizeof(buffer) in this example?

const char buffer[] = "A buffer."

It's not 9, it's 10.

(Short answer: It's a C string. It's null-terminated. So is your data
array.)

-Chad

 

[Alf P. Steinbach]

[Do not top-post. Read the FAQ on that. Top-posting corrected.]

* "Alf P. Steinbach":

FILE* fpOut = fopen(argv[2], "wt");

Here you're opening the file in text mode. In this mode some compiler
and/or operating system dependent translation may take place. In
particular, in your case, a Ctrl Z may be appended at the end as an
end-of-file marker, which seems to be your immediate problem.

[Do not include signature in reply.]

Alf,

It doesn't appear to be appending a CTRL-Z to the end... its sticking an
extra "\x00" on the end.

If it was appending a CTRL-Z, surely it wouldnt show up in my formatted
byte array right?

The output appears to be formatted correctly except for a \x00 at the end.

I have now tested your program with an input file that contains four
characters. It works "correctly" for that case. At the end is appended
only a carriage return and linefeed, as should be for that OS.

Perhaps the problem is in the parsing of the result file?

 

[Alf P. Steinbach]

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

So later, when you want to write out the dll that you have encoded, you
do something like

const char buffer[] =
#include "outputdll.h"

where outputdll.h contains the hex-encoded file, contained in double
quotes. Correct?

Well, to answer your question about the extra byte, answer this
question. What is sizeof(buffer) in this example?

const char buffer[] = "A buffer."

It's not 9, it's 10.

(Short answer: It's a C string. It's null-terminated. So is your data
array.)

You may just be right.

I didn't think to question the assertion that the file itself contained
an extra byte.

Whack! Whack! Whack! Ouch. Me forehead.

 

[Nobody]

Its just supposed to open the .dll file, read it one byte at a time and
format it like:

"\x00\x01\x02" etc

so I can link it into my code... nByteCount at the end comes out to the
correct value, but I get a file thats one byte too big????

So later, when you want to write out the dll that you have encoded, you
do something like

const char buffer[] =
#include "outputdll.h"

where outputdll.h contains the hex-encoded file, contained in double
quotes. Correct?

Well, to answer your question about the extra byte, answer this
question. What is sizeof(buffer) in this example?

const char buffer[] = "A buffer."

It's not 9, it's 10.

(Short answer: It's a C string. It's null-terminated. So is your data
array.)

You may just be right.

I didn't think to question the assertion that the file itself contained
an extra byte.

Whack! Whack! Whack! Ouch. Me forehead.

Aha! you guys nailed it... the encoding of the file was correct... it was
the decoding that was putting in the extra byte.

I had defined it as:

static const BYTE ldr[] =
{
};

BYTE in the Microsoft world defines as 'unsigned char'... I was under the
impression that it would be a byte array and not a string. My mistake...

If I would have formatted the file as:

'\x00', '\x12', '\xff'

or:

0x00, 0x12, 0xff

it would have worked correctly.

I went back and changed the format to be the later..

static const BYTE ldr[] =
{
0x00, 0x12, 0xff
};

and now sizeof(ldr) returns the correct size... thanks for the help