0

I want to implement a function in my program that sends a .txt to my email with some tasks that i have to do in the day. Here's the code:

void txtCreator(){
  /**file.dat and file.txt, respectively**/
  FILE *fp, *fp1;
  /**struct that contain the events**/
  struct evento *display = (struct evento *)malloc(sizeof(struct evento));

  char buffer[48];
  char email_events[] = {"dd_mm.txt"};//filename.txt
  char msg[]={"Nao ha eventos disponiveis para hoje!\n"};
  int count=0;
  time_t rawtime;
  time(&rawtime);
  struct tm timenow = *localtime(&rawtime);
  strftime(buffer, 48, "%d_%m", &timenow);
  fp = fopen(file_name, "rb");
  fp1 = fopen(email_events, "w");
  if(strcmp(buffer, email_events)!=0){  
    strcpy(email_events, buffer);
    while(fread(display, sizeof(struct evento), 1, fp)==1){
      if (feof(fp) || fp==NULL){
        break;
      }
      else if(display->dia==timenow.tm_mday && display->mes==timenow.tm_mon+1){
        fwrite(display, sizeof(struct evento), 1, fp1);
        fprintf(fp1, "%s", "\n");
        count++;
      }
    }
  }
  if(count==0){
    fprintf(fp1, "%s", msg);
  }
  fclose(fp);
  fclose(fp1);
}

Everything is working just fine, but there's two problems:

1-

strcpy(email_events, buffer);

is not working, and:

2-

when i create the .txt file, it shows like that:

test ¹0(¹€(.v™ ™­ °'¹8¹uguese_Brazil.12

it shows the event name (test) correctly, but the date is not working.

I've tried a lot of things, but nothing works.

Sorry for the bad english, not my native language.

Bhargav Rao
  • 50,140
  • 28
  • 121
  • 140
  • `file_name` is never defined, and could you show the definition of `struct evento`? – Schwern Jul 19 '19 at 23:08
  • 2
    `fwrite()` writes the data in the structure to the file as binary data. There's no reason that should be readable as text. If you want to write to the file as text, you need to `fprintf()` each of the structure's elements. – Lee Daniel Crocker Jul 19 '19 at 23:10
  • Is `email_events` supposed to be `"dd_mm.txt"` or is it supposed to contain real numbers? I ask because `strcmp(buffer, email_events)` will never be equal. – Schwern Jul 19 '19 at 23:16

2 Answers2

0

Assuming that you are meaning to copy email_events into your buffer (since you assigned a static string), your strcpy parameters are backwards.

Below is the declaration of strcpy

char *strcpy(char *dest, const char *src);

You probably meant:

strcpy(buffer, email_events);
fiidim
  • 157
  • 8
0

when i create the .txt file, it shows like that:

test ¹0(¹€(.v™ ™­ °'¹8¹uguese_Brazil.12

Let's address this first: you're not writing text to your .txt file. You're writing a struct. It's going to look like garbage.

For example, let's say display->dia is 19. This means the number 19 will be written to the file, not the text 19, the number 19. Read as text, 19 is garbage. 10 is a newline. 65 is A.

If your intent is to dump the structs into a file, assuming struct evento has no pointers, you're good. In fact you probably shouldn't add a newline, it will interfere with reading the file by the size of the struct.

If your intent is to produce a human readable text file, you need to translate each piece of the struct into text. For example, if you wanted to write the day and month as text...

fprintf(fp1, "%d_%d", display->dia, display->mes);

I'll assume that going forward.


strcpy(email_events, buffer); is not working

At first glance it looks like your strcpy is backwards, it's strcpy(src, dest) and presumably you want to copy email_events into buffer: strcpy(buffer, email_events).

Looking further, your code does nothing with either buffer nor email_events after that. The strcpy is pointless.

Going even further, buffer is the month and day like 07_19. email_events is always dd_mm.txt. Those will never match. strcmp(buffer, email_events)!=0 will always be true making the if check pointless.

I'm not sure what the intent of buffer and email_events are, but it appears to be trying to create a filename based on the current date? This can be done much simpler with one better named variable outfile.

time_t rawtime;
time(&rawtime);
struct tm timenow = *localtime(&rawtime);
char outfile[20];
strftime(outfile, 20, "%d_%m.txt", &timenow);

Moving along to other problems, you don't check that fp1 opened.

You do eventually check fp but you check it after you've already read from a possibly null file pointer. If you're compiling with an address sanitizer (which you should) it will cause an error. Causing an error when using a null pointer is good, it will solve many a mystery memory problem for you.

It's much easier and robust and address sanitizer friendly to check immediately. We can also do a better job naming them to avoid confusing the input from the output: in and out.

FILE *in = fopen(file_name, "rb");
if( in == NULL ) {
    perror(file_name);
    exit(1);
}

And since you're reading binary with rb you should be writing binary you should be using wb. This only matters on Windows, but might as well be consistent.

FILE *out = fopen(outfile, "wb");
if( out == NULL ) {
    perror(outfile);
    exit(1);
}

There's no need to check feof(fp), while(fread(display, sizeof(struct evento), 1, fp)==1) will already exit the loop at end of file when it fails to read. In general, explicitly checking for the end of file leads to subtle problems.

The read/write loop is now much simpler.

while(fread(display, sizeof(struct evento), 1, in)==1){
    if(display->dia==timenow.tm_mday && display->mes==timenow.tm_mon+1) {
        fprintf(out, "%d_%d\n", display->dia, display->mes);
        count++;
    }
}

Putting it all together...

void txtCreator(){
    const char *no_events_found_msg = "Nao ha eventos disponiveis para hoje!\n";
    
    // No need to cast the result of malloc, it just invites mistakes.
    struct evento *display = malloc(sizeof(struct evento));
        
    // Generate the output filename directly, no strcmp and strcpy necessary.
    time_t rawtime;
    time(&rawtime);
    struct tm timenow = *localtime(&rawtime);
    char outfile[20];
    strftime(outfile, 48, "%d_%m.txt", &timenow);

    // Immediatetly make sure the files are open and error immediately.
    FILE *in = fopen(file_name, "rb");
    if( in == NULL ) {
        perror(file_name);
        exit(1);
    }
  
    FILE *out = fopen(outfile, "wb");
    if( out == NULL ) {
        perror(outfile);
        exit(1);
    }

    // Now that we know the files are open, reading and writing is much simpler.
    int count=0;
    while(fread(display, sizeof(struct evento), 1, in)==1){
        if(display->dia==timenow.tm_mday && display->mes==timenow.tm_mon+1) {
            fprintf(out, "%d_%d\n", display->dia, display->mes);
            count++;
        }
    }
    
    if(count==0){
        fprintf(out, "%s", no_events_found_msg);
    }
    
    fclose(in);
    fclose(out);
}

Note that I've used a style which declares variables in place. This makes the code easier to read, limits the scope of each variable, and it avoids declaring variables which you never use.

Community
  • 1
  • 1
Schwern
  • 153,029
  • 25
  • 195
  • 336