0

I'm doing a program for a university project. currently I'm doing the user configuration for being able to change both the password and the user name. I was thinking a way to use a single function for both process instead of creating two separated ones.

My goal was to pass a reference to the struct that holds the user data, and a reference to the corresponding member of the struct I want to edit, both being a char[64]. So after I edit the member I can just rewrite the whole line in the file with the updated data.


void change_user_data(char *input, struct user ***logged_user,
                      char *data_to_alter, FILE **user_registry) {
  // making sure the returned input is valid.
  if (strcmp(input, "..."))
    return;

  struct user find_user;

  fseek(*user_registry, 0, SEEK_SET);
  while (!feof(*user_registry)) {
    fread(&find_user, sizeof(struct user), 1, *user_registry);

    if (0 != strcmp(find_user.user_name, (**logged_user)->user_name))
      continue;

    strcpy(data_to_alter, input);

    fseek(*user_registry, 0, SEEK_CUR - 1);
    fwrite((**logged_user), sizeof(struct user), 1, *user_registry);

    break;
  }
  return;
}

This is the function that should handle the change of both the struct and the file.


change_user_data(change_password(input), &logged_user, (*logged_user)->password, &user_id);

and here was my attempt to pass the arguments.

There was no error message, but there wasn't any change neither for the struct or the file. I guess that what is happening is that it's only creating a copy of the member, I tried to change the way I passed the member but none of them worked.

Minimal Reproductable Example:

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

#define MAXLEN 64

struct user {
  char user_name[MAXLEN];
  char password[MAXLEN];
};

void placeholder_function(struct user *logged_user);
void change_user_data(char *input, struct user **logged_user,
                      char *data_to_alter, FILE **user_registry);

int main() {
  struct user logged_user;

  placeholder_function(&logged_user);

  return 0;
}

void placeholder_function(struct user *logged_user) {
  FILE *user_registry;
  if (!(user_registry = fopen("user_registry.bin", "w+b"))) {
    printf("Couldn't open file\n");
    exit(1);
  }
  strcpy(logged_user->user_name, "Admin\0");
  strcpy(logged_user->password, "Admin\0");

  fseek(user_registry, 0, SEEK_SET);
  fwrite(logged_user, sizeof(struct user), 1, user_registry);


  // expected output: Admin Admin
  printf("%s %s\n", logged_user->user_name, logged_user->password);

  // rest of program here...

  change_user_data("1234\0", &logged_user, logged_user->password,
                   &user_registry);

  printf("%s %s\n", logged_user->user_name, logged_user->password);
  // wanted output: Admin 1234
}

void change_user_data(char *input, struct user **logged_user,
                      char *data_to_alter, FILE **user_registry) {
  struct user find_user;

  fseek(*user_registry, 0, SEEK_SET);
  while (!feof(*user_registry)) {
    fread(&find_user, sizeof(struct user), 1, *user_registry);

    if (0 != strcmp(find_user.user_name, (*logged_user)->user_name))
      continue;

    strcpy(data_to_alter, input);

    fseek(*user_registry, 0, SEEK_CUR - 1);
    fwrite((*logged_user), sizeof(struct user), 1, *user_registry);

    break;
  }
}
  • Edit the question to provide a [mre]. – Eric Postpischil Sep 11 '22 at 20:13
  • 1
    Fix [this problem](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) first. – n. m. could be an AI Sep 11 '22 at 20:39
  • Before code not present here _updates_ info contained in a record, the complete record should be retrieved (or not). After validating and accepting new info _into_ that record, the "database" version can be overwritten with the entire record. Trying to "poke" something into a particular field is far more complicated than you want to write at this stage. – Fe2O3 Sep 11 '22 at 21:00
  • @n.1.8e9-where's-my-sharem. Thanks. I'll look in on that and start to implement it on my code – Kuudere-san Sep 11 '22 at 21:36
  • @Fe2O3 I'm sorry, but I can't quite get the idea. Do you mean that the database must be rewritten separately in it's own function? – Kuudere-san Sep 11 '22 at 21:38
  • 1
    @Fe203 I don't know what you did, but what you said caused my two neurons to do synapses... It is probably still garbage code, and probably it is not what you meant. But it definitely works now. Thanks man. Although I still wish to know more. if you have any links about the subject I would appreciate it. – Kuudere-san Sep 11 '22 at 22:05
  • In a simple form, there are three steps: 1) find & retrieve the record of interest; 2) modify field(s) of that record; 3) return the record to persistent storage. You've not shown code for #1, and it appears you want to force part of #2 into step #3. Why? Would you try to insert a CD into its case at the same time you are filing the case back on the shelf? "Process"... A series of independent but related steps performed in a logical sequence. – Fe2O3 Sep 11 '22 at 22:07
  • "Links?" No. Except perhaps the kindergarten rhyme: "Aye Bee Cee Dee E Ef Gee"... Do things in order and there will be order... Glad you've got it working... `:-)` – Fe2O3 Sep 11 '22 at 22:12
  • One more 'tip'... The more you can simplify and "isolate" a particular operation, the easier it is to design & code & test that the desired function works before integrating that function into a larger program... Blurred interdependencies are are a nightmare... – Fe2O3 Sep 11 '22 at 22:18
  • 1
    @Fe2O3 Thanks, I'll remember that rhyme, It seems I had forgotten it. – Kuudere-san Sep 11 '22 at 22:27

2 Answers2

0

I'm not 100 % sure what's going wrong in your specific example.

But you only have to pass the address of the struct or the address of the element, not both. In this case:

struct user ***logged_user

Then you can modify the element with:

strcpy((**logged_user)->password, input);

Now if you would only care about the password and not the username in this function, you could have passed in a pointer to only the password, and not the container struct. But that's your choice.

Now there are some other concerns with this code. You need to make sure the copy from input doesn't cause a buffer overrun in (**logged_user)->password. You can either do this by making sure the input is small enough, or using the strncpy function instead. Make sure destination becomes null terminated, typically with (**logger_user)->password[pass_len - 1] = '\0'

This line fseek(*user_registry, 0, SEEK_CUR - 1);

Is weird, SEEK_CUR - 1 will evaluate to SEEK_SET, which I doubt you want to do.

Tor
  • 1
  • Hi, I'm currently doing minimal reproducible example as requested by a commentator. I had originally made a function to change the user name and another to change the password and both worked. But after I noticed that both used the same logic and the only difference was the pointed member I wanted to try and unify both. – Kuudere-san Sep 11 '22 at 20:49
  • The placement of the '\0' had already made it previously while getting the user input. – Kuudere-san Sep 11 '22 at 20:52
  • I didn't knew about that SEEK_CUR interaction. Currently the program only has one default user, so that explains why it works currently. I'm quite new to programing and I have probably made a lot of wtf things in this project. What would you recommend to change the line in the file I had just passed? – Kuudere-san Sep 11 '22 at 20:52
  • At the end I used the first thing on another function. I still need a way to go to the previous line, would simply doing `-1` do that? – Kuudere-san Sep 11 '22 at 22:33
0

Well, It works now. Just changed the edit of the struct to the function change_password(). And edited the exact member i wanted to edit.


bool change_password(char *new_password, struct user ***logged_user) {
  char repeat_password[MAXLEN];

  system(CLEAR);

  printf("\nInput the new password: ");
  trim_line(get_input(new_password, MAXLEN));

  printf("\nRepeat the new password: ");
  trim_line(get_input(repeat_password, MAXLEN));

  if (0 != strcmp(new_password, repeat_password)) {
    fprintf(stderr, "\nNew password mismatch!\n");
    printf(KEY_PRESS);
    free_buffer();
    return false;
  }

  strcpy((**logged_user)->data.password, new_password);
  return true;
}

After that I pass the struct to the change_user_data() function and done

void change_user_data(bool is_valid, struct user ***logged_user,
                      FILE **user_registry) {
  // making sure the returned input is valid.
  if (!is_valid)
    return;

  struct user find_user;

  fseek(*user_registry, 0, SEEK_SET);
  while (!feof(*user_registry)) {
    fread(&find_user, sizeof(struct user), 1, *user_registry);

    if (0 != strcmp(find_user.data.user_name, (**logged_user)->data.user_name))
      continue;

    fseek(*user_registry, 0, SEEK_CUR - 1);
    fwrite((**logged_user), sizeof(struct user), 1, *user_registry);

    break;
  }
  return;
}

I still need to implement the other advice I was given, like not using feof(), and placing myself correctly in the file, but it's a start.

  • "Finding file position to overwrite". Use `ftell( )` ahead of every record "read". When you've found target record, THAT is the position to return to when you want to replace old with new. (Maybe a global `size_t` variable, initially, just to get it working. Then, a mechanism that doesn't require using global variables.) btw: `fseek( ... SEEK_CUR )` with negative offset can lead to "undefined behaviour" I've been told... – Fe2O3 Sep 11 '22 at 22:23
  • 1
    @Fe2O3 Mmm... I didn't kwen about `ftell()` I'll implement it straight away. Thanks again! – Kuudere-san Sep 11 '22 at 22:40
  • Read man pages instead of coding... 10 minutes of reading will save an hour of coding (often reinventing the wheel... and doing a poor job of it...) `:-)` – Fe2O3 Sep 11 '22 at 22:42
  • 1
    @Fe2O3 `,:D` You're right – Kuudere-san Sep 11 '22 at 22:57