Do "optimizations" really optimize?

Page 1 of 1 [ 11 posts ]

Joined: 3 Jul 2005
Age: 39
Gender: Male
Posts: 511
Location: Bathurst, Australia

11 Apr 2008, 12:07 am

Hey,

I wrote a program to implement a 35 step secure file erasing algorithm - see here - and for the life of me, I couldn't get it to overwrite the file - it kept adding to it. So I was advised to change it to r+ but it still didn't work - the pointer was at the beginning of the file and I was using the current location straight after opening to check for the file size and it died right there, so I made it seek to the end of the file before calling ftell() on the file pointer.

After that was fixed I was advised to create a function pointer array to store each step and loop through it. The way I had it had a separate section for it - the same code repeated 29 times (the random parts were looped). So I followed that advice and here are my results.

Both versions of the program use this - erase-types.c

Code:

#include <stdio.h>
#include <stdlib.h>

static FILE *urandom;

static char chrbin[4];

void start_random() {
   urandom = fopen("/dev/urandom", "r");
}
void stop_random() {
   fclose(urandom);
}

char *random_str() {   

   sprintf(chrbin, "%c", fgetc(urandom));
   return chrbin;
}
char *x92_49_24() {
   sprintf(chrbin, "%c%c%c", 0x92, 0x49, 0x24);
   return chrbin;
}
char *x49_24_92() {
   sprintf(chrbin, "%c%c%c", 0x49, 0x24, 0x92);
   return chrbin;
}
char *x24_92_49() {
   sprintf(chrbin, "%c%c%c", 0x24, 0x92, 0x49);
   return chrbin;
}
char *x00() {
   sprintf(chrbin, "%c", 0x00);
   return chrbin;
}
char *x11() {
   sprintf(chrbin, "%c", 0x11);
   return chrbin;
}
char *x22() {
   sprintf(chrbin, "%c", 0x22);
   return chrbin;
}
char *x33() {
   sprintf(chrbin, "%c", 0x33);
   return chrbin;
}
char *x44() {
   sprintf(chrbin, "%c", 0x44);
   return chrbin;
}
char *x55() {
   sprintf(chrbin, "%c", 0x55);
   return chrbin;
}
char *x66() {
   sprintf(chrbin, "%c", 0x66);
   return chrbin;
}
char *x77() {
   sprintf(chrbin, "%c", 0x77);
   return chrbin;
}
char *x88() {
   sprintf(chrbin, "%c", 0x88);
   return chrbin;
}
char *x99() {
   sprintf(chrbin, "%c", 0x99);
   return chrbin;
}
char *xAA() {
   sprintf(chrbin, "%c", 0xAA);
   return chrbin;
}
char *xBB() {
   sprintf(chrbin, "%c", 0xBB);
   return chrbin;
}
char *xCC() {
   sprintf(chrbin, "%c", 0xCC);
   return chrbin;
}
char *xDD() {
   sprintf(chrbin, "%c", 0xDD);
   return chrbin;
}
char *xEE() {
   sprintf(chrbin, "%c", 0xEE);
   return chrbin;
}
char *xFF() {
   sprintf(chrbin, "%c", 0xFF);
   return chrbin;
}
char *x6D_B6_DB() {
   sprintf(chrbin, "%c%c%c", 0x6D, 0xB6, 0xDB);
   return chrbin;
}
char *xB6_DB_6D() {
   sprintf(chrbin, "%c%c%c", 0xB6, 0xDB, 0x6D);
   return chrbin;
}
char *xDB_6D_B6() {
   sprintf(chrbin, "%c%c%c", 0xDB, 0x6D, 0xB6);
   return chrbin;
}

Here's erase-types.h

Code:

char *random_str();
void start_random();
void stop_random();
char *x92_49_24();
char *x49_24_92();
char *x24_92_49();
char *x00();
char *x11();
char *x22();
char *x33();
char *x44();
char *x55();
char *x66();
char *x77();
char *x88();
char *x99();
char *xAA();
char *xBB();
char *xCC();
char *xDD();
char *xEE();
char *xFF();
char *x6D_B6_DB();
char *xB6_DB_6D();
char *xDB_6D_B6();

And main.c - of the first version

Code:

#include <stdio.h>
#include <stdlib.h>
#include "erase_types.h"

int main(int argc, char **argv) {

   char *erchar;
   int iter;
   int chrLoc;
   long filesize;
   FILE *out_fp;

   

   if (argc != 2) {

      fprintf(stderr, "Syntax: %s <file>\n", argv[0]);
      exit(1);
   }

   if ((out_fp = fopen(argv[1], "r+")) == NULL) {

      fprintf(stderr, "Error: Unable to open file %s for writing\n", argv[1]);
   }

   start_random();

   fseek(out_fp, 0, SEEK_END);
   filesize=ftell(out_fp);

   printf("size: %d\n", filesize);

   /* 1-4. random run */
   for (iter=0; iter < 4; iter++) {

      fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

      for (chrLoc = 0; chrLoc < filesize; chrLoc++) {

         fputs(random_str(), out_fp);
      }
   }

   /* 5. alternating one pattern (01010101) */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x55(), out_fp);
   }

   /* 6. alternating one pattern (10101010) */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xAA(), out_fp);
   }

   /* 7. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x92_49_24(), out_fp);
   }
   /* 8. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x49_24_92(), out_fp);
   }
   /* 9. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x24_92_49(), out_fp);
   }

   /* 10. 0x00 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x00(), out_fp);
   }
   /* 11. 0x11 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x11(), out_fp);
   }
   /* 12. 0x22 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x22(), out_fp);
   }
   /* 13. 0x33 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x33(), out_fp);
   }
   /* 14. 0x44 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x44(), out_fp);
   }
   /* 15. 0x55 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x55(), out_fp);
   }
   /* 16. 0x66 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x66(), out_fp);
   }
   /* 17. 0x77 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x77(), out_fp);
   }
   /* 18. 0x88 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x88(), out_fp);
   }
   /* 19. 0x99 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x99(), out_fp);
   }
   /* 20. 0xAA pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xAA(), out_fp);
   }
   /* 21. 0xBB pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xBB(), out_fp);
   }
   /* 22. 0xCC pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xCC(), out_fp);
   }
   /* 23. 0xDD pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xDD(), out_fp);
   }
   /* 24. 0xEE pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xEE(), out_fp);
   }
   /* 25. 0xFF pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xFF(), out_fp);
   }

   /* 26. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x92_49_24(), out_fp);
   }
   /* 27. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x49_24_92(), out_fp);
   }
   /* 28. (2,7) RLL and MFM pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x24_92_49(), out_fp);
   }

   /* 29. (2,7) RLL pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(x6D_B6_DB(), out_fp);
   }
   /* 30. (2,7) RLL pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(xB6_DB_6D(), out_fp);
   }
   /* 31. (2,7) RLL pattern wiping - 1 */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc+=3) {
      fputs(xDB_6D_B6(), out_fp);
   }

   /* 32-35. random run */
   for (iter=0; iter < 4; iter++) {

      fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

      for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
         fputs(random_str(), out_fp);
      }
   }

   stop_random();

   fclose(out_fp);
#ifndef DEBUG
   remove(argv[1]);
#endif

   printf("\n");
}

Main2.c - of the second version

Code:

#include <stdio.h>
#include <stdlib.h>
#include "erase_types.h"
#include "main2.h"

int main(int argc, char **argv) {

   sequence seqs[36]={NULL};
   int seqcount = -1;
   int iter;
   int chrLoc;
   long filesize;
   FILE *out_fp;

   if (argc != 2) {

      fprintf(stderr, "Syntax: %s <file>\n", argv[0]);
      exit(1);
   }

   if ((out_fp = fopen(argv[1], "r+")) == NULL) {

      fprintf(stderr, "Error: Unable to open file %s for writing\n", argv[1]);
   }

   start_random();

   fseek(out_fp, 0, SEEK_END);
   filesize=ftell(out_fp);

   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&x92_49_24;
   seqs[++seqcount]=&x49_24_92;
   seqs[++seqcount]=&x24_92_49;
   seqs[++seqcount]=&x00;
   seqs[++seqcount]=&x11;
   seqs[++seqcount]=&x22;
   seqs[++seqcount]=&x33;
   seqs[++seqcount]=&x44;
   seqs[++seqcount]=&x55;
   seqs[++seqcount]=&x66;
   seqs[++seqcount]=&x77;
   seqs[++seqcount]=&x88;
   seqs[++seqcount]=&x99;
   seqs[++seqcount]=&xAA;
   seqs[++seqcount]=&xBB;
   seqs[++seqcount]=&xCC;
   seqs[++seqcount]=&xDD;
   seqs[++seqcount]=&xEE;
   seqs[++seqcount]=&xFF;
   seqs[++seqcount]=&x92_49_24;
   seqs[++seqcount]=&x49_24_92;
   seqs[++seqcount]=&x24_92_49;
   seqs[++seqcount]=&x6D_B6_DB;
   seqs[++seqcount]=&xB6_DB_6D;
   seqs[++seqcount]=&xDB_6D_B6;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;
   seqs[++seqcount]=&random_str;

   seqcount++;

   for (iter = 0; iter < seqcount; iter++) {

      for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
         fseek(out_fp, 0, SEEK_SET); /* back to the beginning */
      }
      fputs((*seqs[iter])(), out_fp);
   }

   stop_random();

   fclose(out_fp);

#ifndef DEBUG
   remove(argv[1]);
#endif

   return 0;
}

and main2.h:

Code:

typedef char *(*sequence)();

The Makefile that put it all together:

Code:

TARGET=firmerase
TARGET2=firmerase2
OFILES=erase_types.o main.o
OFILES2=erase_types.o main2.o
CFILES=erase_types.c main.c
CFILES2=erase_types.c main2.c
#CC=gcc
CC=cc
CFLAGS=-O2
#LDFLAGS=-lwin32k
LDFLAGS=
#STRIP=--strip-all
STRIP=

all: $(TARGET) $(TARGET2)

$(TARGET): $(OFILES)
   $(CC) -o $(TARGET) $(OFILES) $(LDFLAGS)
   strip $(STRIP) $(TARGET)

$(TARGET2): $(OFILES2)
   $(CC) -o $(TARGET2) $(OFILES2) $(LDFLAGS)
   strip $(STRIP) $(TARGET2)

erase_types.o:

main.o: erase_types.h

main2.o: erase_types.h main2.h

clean:
   rm -f $(TARGET) $(OFILES) $(OFILES2)

And here are the execution results:

Code:

bash-3.00$ ls -l firmerase*
-rwxr-xr-x   1 amason09 student    10304 Apr 11 13:34 firmerase*
-rwxr-xr-x   1 amason09 student     8792 Apr 11 13:39 firmerase2*
bash-3.00$ cat main.c > main.c.orig
bash-3.00$ time ./firmerase main.c.orig
size: 5644


real    0m0.073s
user    0m0.062s
sys     0m0.010s
bash-3.00$ cat main.c > main.c.orig
bash-3.00$ time ./firmerase2 main.c.orig

real    0m0.377s
user    0m0.221s
sys     0m0.134s

The second version was less than 1.5kb smaller, but took 5 times longer to do the same thing.

This was executed on a Sun Fire V210 running SunOS 5.10 and with 4GB RAM.

The following test was done on a Dual Pentium 2.8 with 256mb RAM available (VPS):

Code:

milo:~/dev/firmerase-0.1# ls -l firmerase*
-rwxr-xr-x 1 root root 7480 Apr 11 15:00 firmerase
-rwxr-xr-x 1 root root 5720 Apr 11 15:00 firmerase2
milo:~/dev/firmerase-0.1# cat main.c > main.c.orig
milo:~/dev/firmerase-0.1# time ./firmerase main.c.orig
size: 5626


real    0m0.112s
user    0m0.040s
sys     0m0.013s
milo:~/dev/firmerase-0.1# cat main.c > main.c.orig
milo:~/dev/firmerase-0.1# time ./firmerase2 main.c.orig

real    0m0.364s
user    0m0.052s
sys     0m0.067s

This time, more than 3 times longer, for a 1,760 byte space saving.

A lot of emphasis is put on optimizing code for readability and less code repetition, but is it really that beneficial for performance?

_________________
We are one, we are strong... the more you hold us down, the more we press on - Creed, "What If"

AS is definitive. Reality is frequently inaccurate.

I'm the same as I was when I was six years old - Modest Mouse

Warsie
Veteran
Veteran

Joined: 3 Apr 2008
Age: 34
Gender: Male
Posts: 2,542
Location: Chicago, IL, USA

11 Apr 2008, 10:50 am

to be honest-I have no answer to this. I Thought this was dealing with whether stuff like Windows Optimizer was good or not.

regarding your question...use Tracks eraser Pro to erase.

_________________
I am a Star Wars Fan, Warsie here.
Masterdebating on chi-city's south side.......!

Betzalel
Deinonychus

Joined: 22 Feb 2008
Age: 44
Gender: Male
Posts: 317

11 Apr 2008, 10:51 am

without trying to run this code myself I couldnt say where the bottleneck is but I think it woudl be educational to run it through a profiler to see where in the code takes up the most time.

the most common UNIX tools for this are prof and gprof and require you to compile the code with the right flags given to the C compiler to insert the profiling code that gerates the profiling data file while the compiled program executes.

the prof and gprof tools then read this file and tell you the statistics for each line of code that was run.

Aaron_Mason
Veteran
Veteran

Joined: 3 Jul 2005
Age: 39
Gender: Male
Posts: 511
Location: Bathurst, Australia

11 Apr 2008, 4:57 pm

We can disregard this... I found a bug in the second program that resulted in it doing next to nothing and taking longer to do it.

The execution times are on par now - in fact sometimes the second program uses slightly less user time.

Here are the execution times on my devel box - Athlon 1.8GHz, 512mb RAM:

Code:

[root@togepi firmerase-0.1]# ls -l firmerase*
-rwxr-xr-x 1 root root 7128 2008-04-12 07:45 firmerase
-rwxr-xr-x 1 root root 5792 2008-04-12 07:45 firmerase2
[root@togepi firmerase-0.1]# cp main.c{,.orig}
[root@togepi firmerase-0.1]# time ./firmerase main.c.orig

real    0m0.056s
user    0m0.050s
sys     0m0.000s
[root@togepi firmerase-0.1]# cp main.c{,.orig}
[root@togepi firmerase-0.1]# time ./firmerase2 main.c.orig

real    0m0.059s
user    0m0.050s
sys     0m0.010s

Have a look at the code for the second run, and you'll see I made it look doing continual fseeks. - and for longer strings it was still iterating one character at a time.

Run times on the uni's student devel platform - a Sun Fire V210:

Code:

bash-3.00$ time ./firmerase main.c.orig

real    0m0.073s
user    0m0.062s
sys     0m0.011s
bash-3.00$ cp main.c{,.orig}
bash-3.00$ time ./firmerase2 main.c.orig

real    0m0.074s
user    0m0.063s
sys     0m0.011s
bash-3.00$ ls -l firmerase*
-rwxr-xr-x   1 amason09 student    10304 Apr 12 07:48 firmerase*
-rwxr-xr-x   1 amason09 student     9076 Apr 12 07:48 firmerase2*

And on my VPS - Dual Pentium 4 2.8 w/256mb ram available:

Code:

milo:~/dev/firmerase-0.1# ls -l firmerase*
-rwxr-xr-x 1 root root 7268 Apr 12 07:53 firmerase
-rwxr-xr-x 1 root root 6104 Apr 12 07:53 firmerase2
milo:~/dev/firmerase-0.1# cp main.c{,.orig}
milo:~/dev/firmerase-0.1# time ./firmerase main.c.orig

real    0m0.055s
user    0m0.046s
sys     0m0.009s
milo:~/dev/firmerase-0.1# cp main.c{,.orig}
milo:~/dev/firmerase-0.1# time ./firmerase2 main.c.orig

real    0m0.066s
user    0m0.046s
sys     0m0.009s

Case closed. The code is available here (right click and save as - webserver's being strange)

lau
Veteran
Veteran

Joined: 17 Jun 2006
Age: 75
Gender: Male
Posts: 9,795
Location: Somerset UK

11 Apr 2008, 8:27 pm

What staggeringly awful code.

Why call a subroutine to set up each string each time, when most of the time the string is constant.

Storing a string in a global variable and returning a pointer to that is insane.

Storing a string in a global variable and returning a pointer to that is insane.

I had to say that twice.

Repeating the loop according to the file size, but writing strings that are three characters long (sometimes) should triple the file size.

The "random" output can store a zero byte as its value, hence the fputs will not output anything. I.e. the random loop will expect to overwrite only 255/256 of the file.

Assuming that simple stream writes with fputs will reach the disc is wrong. System buffering, of one sort or another, will usually mean that only the last pass through the file actually writes to the physical disk.

Even using "write" won't help. You must ensure that the data is flushed to the drive. This will be platform dependent. It may even be drive hardware dependent.

You seem to have missed out a "typedef" for "sequence".

I didn't look for any more problems.

_________________
"Striking up conversations with strangers is an autistic person's version of extreme sports." Kamran Nazeer

wolphin
Velociraptor

Joined: 15 Aug 2007
Age: 37
Gender: Male
Posts: 465

11 Apr 2008, 11:51 pm

Your code is going to end up being disk bound anyways. You'll see diminishing returns from optimization.

If you want more performance, possibly there are some tricks like turning off disk caching or such.

Speaking of disk caching, this program probably won't do want you intend. On top of the operating system caching, the disk probably has internal cache ram to buffer your writes. Therefore even though it seems like you're overwriting things multiple times, it might in fact actually be overwriting things less than that.

Aaron_Mason
Veteran
Veteran

Joined: 3 Jul 2005
Age: 39
Gender: Male
Posts: 511
Location: Bathurst, Australia

11 Apr 2008, 11:52 pm

lau wrote:

Repeating the loop according to the file size, but writing strings that are three characters long (sometimes) should triple the file size.

That code is out of date. I fixed that bug. If you read my last post you'd know that.

lau wrote:

You seem to have missed out a "typedef" for "sequence".

Look in main2.h.

Thank you for a well-rounded critique of my code. Also, thank you for pointing out my program's flaws. I will take them into account.

lau
Veteran
Veteran

Joined: 17 Jun 2006
Age: 75
Gender: Male
Posts: 9,795
Location: Somerset UK

12 Apr 2008, 8:26 am

Aaron_Mason wrote:

lau wrote:

Repeating the loop according to the file size, but writing strings that are three characters long (sometimes) should triple the file size.

That code is out of date. I fixed that bug. If you read my last post you'd know that.

Nope. Not quite. Just doing "+=3" doesn't help a lot, as it assumes that the file length is a multiple of three.

Also, it does not fix the bug with the random data at all, as the length (0 or 1) of EVERY call via the function pointer must be checked. Making a initial call to generate one byte of data (which may be a zero) will certainly ensure that "es_len" is never set to zero. However, all the subsequent calls may generate zero length strings (1 in 256 times), so exactly the same 255/256 of the file will be written. "fputs" will NEVER write a zero byte. (PS this is not a "race condition".)

And... another comment on the random stuff... acquiring bulk random data by using /dev/urandom is an AWFULLY bad idea. To quote the man page: "/dev/random should be suitable for uses that need very high quality randomness such as one-time pad or key generation." The emphasis here is on "one-time". Read the man page. You can seed the random number generator from there, but the "entropy pool" it uses will run out quite quickly. Try calling "makepasswd" repeatedly. You'll see it pump out a bunch of passwords, but then it grinds to a crawl, while it waits for "environmental noise".

In fact, I wonder if your timings reflect anything but the cost of dev/urandom data.

Aaron_Mason wrote:

lau wrote:

You seem to have missed out a "typedef" for "sequence".

Look in main2.h.

Ah. I didn't notice that little file.

Aaron_Mason wrote:

Thank you for a well-rounded critique of my code. Also, thank you for pointing out my program's flaws. I will take them into account.

I get quite indulgent with my makefiles. E.g.

minimally, CFLAGS wrote:

-Wall -Wstrict-prototypes

and often, CFLAGS wrote:

-Wall -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wunused-parameter -Wunknown-pragmas -Wextra -Wdeclaration-after-statement -Wshadow -Wunsafe-loop-optimizations -Wpointer-arith -Wbad-function-cast -Wc++-compat -Wcast-qual -Wcast-align -Wwrite-strings -Wconversion -Wsign-compare -Waggregate-return -Wold-style-definition -Wmissing-field-initializers -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Wunreachable-code

I.e. I get the compiler to do all the tedious work of spotting my mistakes.

The "strict prototypes" gives lots of warnings. However, there are three "Wall" warnings, all in main.c: a mismatch in printf format (no big), an unused variable (erchar), and the missing return value at the end of main (which means it will return the value of the final printf, which had no arguments printed, so will fortuitously be zero).

_________________
"Striking up conversations with strangers is an autistic person's version of extreme sports." Kamran Nazeer

olle
Snowy Owl

Joined: 2 Mar 2008
Age: 31
Gender: Male
Posts: 157

12 Apr 2008, 6:28 pm

I'm not a very good programmer myself, but even I saw that this code was pretty rotten. I don't mean to be insulting, I'm just a bit honest.

Lau pointed out lots of errors, some of which I didn't see, but here are a few more:

==erazortypes.c==

You call sprintf to fill a string with one single character, even A CONSTANT CHARACTER! I don't think the compiler knows how to get rid of that, and replace it with something more efficient. sprintf == (calling a function, reading control string, reading the variadic argument, writing to the target string, terminating the target string, returning) == slooooow. Using strcpy is faster.

strcpy (chrbin, "\x66");
Or even
chrbin[0]=0x66;
chrbin[1]=0;
which is faster than strcpy.

==main2==
You iterate through an array (seqs[]) and fill each element with a constant. At the start of a function, you try to LOOP through an array to fill it with CONSTANTS! That's what initiators are used for.
Initiate seqs:
sequence seq[] = {
&random_str,
&random_str,
&random_str,
and so on. It's possible that GCC can optimize that part of your code away though.
Afterwards you can use sizeof (array)/sizeof(char*) to get the number of elements.

Code:

      char* newstr;

      fseek(out_fp, 0, SEEK_SET); /* back to the beginning */
      for (chrLoc = 0; chrLoc < filesize; chrLoc+=es_len) {
         newstr = (*seqs[iter])();
         es_len=strlen(newstr);

         fputs(newstr, out_fp);
      }

Get a new string, write it, and step as many bytes forward as there was bytes written.

Actually, the easiest and fastest would probably be to do as you do now, but in the random_str function loop while you get a zero from the /dev/urandom. Problem solved. At least one of them.
lau is right, the file size will be a multiple of 3, but it's not like it's any big problem. Two bytes are wasted, so what.
(lau, you do know that it will not stop writing until after it has passed "filesize" already, right?)

wolphin
Velociraptor

Joined: 15 Aug 2007
Age: 37
Gender: Male
Posts: 465

12 Apr 2008, 7:19 pm

In terms of ultimate speed, I do agree that:

Code:

chrbin[0]=0x66;
chrbin[1]=0;

will be faster than:

Code:

sprintf(chrbin, "%c", 0x66)

However, like I said, if you were to actually measure the difference in speed it would be minimal. This program is disk bound anyway (or else bound on /dev/urandom like mentioned above, or if the OS/disk caches the changes so nothing actually gets written to disk anyway)

Also, perhaps this is being pedantic and/or overcautious, but I'm tempted to point out that using either sprintf and strcpy could possibly lead to problems. Instead snprintf and strncpy should be used in general since they allow you to (somewhat) protect against buffer overflow issues. There's no visible problems in this code, though, in terms of that.

My biggest suggestion for your code is not to optimize but to consolidate/generalize/eliminate all those extra functions. Instead of having a bunch of functions like:

Code:

char *xBB() {
   sprintf(chrbin, "%c", 0xBB);
   return chrbin;
} 

have something along the lines of:

Code:

char* write_byte(char* chrbin, char b){
    chrbin[0] = b;
    chrbin[1] = 0;
    return chrbin;
}

and then anywhere you have:

Code:

fputs(xAA(), out_fp);

replace it with:

Code:

char chrbin[2];
fputs( write_byte(chrbin, 0xAA), out_fp);

and in fact the whole block of code here:

Code:

   /* 10. 0x00 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x00(), out_fp);
   }
   /* 11. 0x11 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x11(), out_fp);
   }
   /* 12. 0x22 pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(x22(), out_fp);
   }
   ...
   /* 24. 0xEE pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xEE(), out_fp);
   }
   /* 25. 0xFF pattern */
   fseek(out_fp, 0, SEEK_SET); /* return to the beginning */

   for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
      fputs(xFF(), out_fp);
   }

can be replaced by:

Code:

for (int i = 0, char c = 0x00; i < 16; i++, c += 0x11){
    fseek(out_fp, 0, SEEK_SET);
    for (chrLoc = 0; chrLoc < filesize; chrLoc++) {
       char chrbin[2];
       chrbin[0] = c;
       chrbin[1] = 0;
       fputs(chrbin, out_fp);
    }
}

Since this has a much higher chance of fitting in CPU cache, the performance will probably be higher. Also (much more importantly) it is more readable, more maintainable, and more compact.

EDIT: Oops, made a mistake in the for loop.

lau
Veteran
Veteran

Joined: 17 Jun 2006
Age: 75
Gender: Male
Posts: 9,795
Location: Somerset UK

12 Apr 2008, 7:31 pm

olle wrote:

All very true, but the aim of the code was obviously to make each pass identical, other than a change to the bottom level routine invoked to generate the content of the file.
This is truly an awful way of doing this particular program, but the concept is quite good, for other situations.

olle wrote:

strcpy (chrbin, "\x66");
Or even
chrbin[0]=0x66;
chrbin[1]=0;
which is faster than strcpy.

==main2==
You iterate through an array (seqs[]) and fill each element with a constant. At the start of a function, you try to LOOP through an array to fill it with CONSTANTS! That's what initiators are used for.
Initiate seqs:
sequence seq[] = {
&random_str,
&random_str,
&random_str,
and so on. It's possible that GCC can optimize that part of your code away though.

There is a major problem here, which Aaron_Mason may have run into. Different compilers have terribly different ideas about what exactly you can use to initialise an array of pointers to functions.
Some require the ampersand, as you have shown, and some refuse to accept it. In one case, IIRC, I had to use a forced cast on each initialiser to browbeat the compiler into accepting it.

olle wrote:

Afterwards you can use sizeof (array)/sizeof(char*) to get the number of elements.

Actually, this is an interesting one. On some processors, a pointer to a vector to a routine may not be the same as a character pointer (data/program separation). I always use the macro:

Code:

#define lenof(x) (sizeof x/sizeof*x)

olle wrote:

Code:

      char* newstr;

      fseek(out_fp, 0, SEEK_SET); /* back to the beginning */
      for (chrLoc = 0; chrLoc < filesize; chrLoc+=es_len) {
         newstr = (*seqs[iter])();
         es_len=strlen(newstr);

         fputs(newstr, out_fp);
      }

Get a new string, write it, and step as many bytes forward as there was bytes written.

And... one more bit, just before the "fputs"...

Code:

if (chrLoc + es_len > filesize) {
 es_len = filesize - chrLoc;
 newstr[es_len] = 0;
}

although I'd rearrange it all much more prettily, and I would NEVER think of using "fputs", because that's altogether the WRONG function, as it can NEVER write a null byte.

olle wrote:

Actually, the easiest and fastest would probably be to do as you do now, but in the random_str function loop while you get a zero from the /dev/urandom. Problem solved. At least one of them.
lau is right, the file size will be a multiple of 3, but it's not like it's any big problem. Two bytes are wasted, so what.

NO. What is happening is that the file size will be FORCED UP to a multiple of three.

olle wrote:

(lau, you do know that it will not stop writing until after it has passed "filesize" already, right?)

No. Reached, or passed. It a "<" in the "for" condition.

Anyway. This program just doesn't do its job. About the only way to ensure that it did something reasonably close to its intended function would be to have some way that one could be certain the data had been flushed to disk after each pass.

To do this on a small file would be impossible, in isolation. One needs to have a file that is at minimum larger than real RAM, I would think, to ensure that the OS did not cache all the writes (write behind).

Even then, the write behind strategy would need to be guaranteed to be "flush out oldest first". If the strategy ever deviated from that, you are still in trouble. E.g. if the file is fragmented across the disc, the strategy could be "flush out buffers nearest to current head location" - a more effective algorithm, but one that defeats the program.

This is why a lot of these "security erase" programs are stand-alone DOS programs. They have much less OS shenanigans to work past.

_________________
"Striking up conversations with strangers is an autistic person's version of extreme sports." Kamran Nazeer

Page 1 of 1 [ 11 posts ]

Jump to:

Do "optimizations" really optimize? Post Reply New Topic

Do "optimizations" really optimize?