fix(index): store inodes unsigned in incremental update snapshots

[sundereshwar]

st_ino is unsigned 64 bit but snapshot.c bound it with sqlite3_bind_int64, so any inode >= 2^63 landed in the TEXT columns as negative decimal. reindex_dir() names the update db in the parking lot with the unsigned value while update_db() looks it up using the string from the snapshot, so the two never matched. stat() returned ENOENT, which is treated as "nothing to install" because that is the normal case for non-suspect directories, and the update was dropped silently with exit 0.

gen_index_snapshot.c also read summary.pinode with %lld, which saturates to LLONG_MAX for parent inodes >= 2^63 and made the diff move unchanged directories through the parking lot for no reason.

Format and bind both the way dbutils.c already writes entries.inode.

BeeGFS inodes are 64 bit hashes, so roughly half of all directories hit this. It does not reproduce on ext4/xfs, where inodes are small sequential numbers.

[manishasalve]

Is this change necessary ? Also here even if first sscanf failed , assigning value in work->pinode on line 98.
I think we can keep the original change as it is ?

[sundereshwar]

This code change started as an effort to change the lld to ino_t type, since it also caused the issue of changing into the wrong inode number.

and after that the line work->pinode stores it as a long long int which makes the inode number a negative value but when we parse it in the snapshot.c below as PRIu64 it goes back to the original number. But wanted to make it correct.

Example:

text from summary.pinode : 14860259768603292038

OLD  sscanf("%lld")            -> 9223372036854775807
         render back with PRIu64   -> 9223372036854775807

NEW sscanf("%" STAT_ino)      -> 14860259768603292038
         stored in long long field -> -3586484305106259578   (looks negative)
         render back with PRIu64   -> 14860259768603292038

I can either just change the %lld to %, Stat_ino and let the negative number conversion happen since it does no harm, but if we want to correct that, then maybe add a protection for the short circuiting

      ino_t pinode = 0;
      const int ok = (sscanf(data[0], "%" STAT_ino, &work->statuso.st_ino) == 1) &&
                     (sscanf(data[1], "%" STAT_ino, &pinode)               == 1);
      if (ok) {
          work->pinode = pinode;
      }
      return !ok;

[manishasalve]

Ok thanks. I earlier missed the replace of %lld with STAT_ino , i think just changing STAT_ino should be enough then rather than adding new variable for return code and check it again ?

return !((sscanf(data[0], "%" STAT_ino, &work->statuso.st_ino) == 1) &&
             (sscanf(data[1], "%" STAT_ino,       &work->pinode)         == 1));

[sundereshwar]

Tried this, doesn't build cause work->pinode is long long int and STAT_ino iss lu on Linux 😬

 src/gufi_incremental_update/gen_index_snapshot.c:95:31: error: format ‘%lu’ expects
  argument of type ‘long unsigned int *’, but argument 3 has type ‘long long int *’
  [-Werror=format=]
     95 |              (sscanf(data[1], "%" STAT_ino, &work->pinode)         == 1));
        |                               ^~~           ~~~~~~~~~~~~~

Would this work if we don't want mess with the return code?

ino_t pinode = 0;
      if ((sscanf(data[0], "%" STAT_ino, &work->statuso.st_ino) != 1) ||
          (sscanf(data[1], "%" STAT_ino, &pinode)               != 1)) {
          return 1;
      }

      work->pinode = pinode;
      return 0;