Compare directory entries (sort by access date)

Question

I have a comparison function for use in scandir or qsort to sort an array of struct dirent *s by access date. S is the directory path of the directory whose files are being sorted, and SavesLen is strlen(S).

static int Q(const struct dirent **const First, const struct dirent **const Second) {
    const char *const FirstName = (*First)->d_name, *const SecondName = (*Second)->d_name, *const Name[] = {FirstName, SecondName};
    const unsigned short FirstLen = (unsigned short)strlen(FirstName), SecondLen = (unsigned short)strlen(SecondName), Len[] = {FirstLen, SecondLen};
    char File[SavesLen+Len[FirstLen < SecondLen]+2], *CurrPtr = memcpy(File, S, SavesLen)+SavesLen;
    *CurrPtr++ = '/';
    time_t Time[2];
    for (unsigned char I = 0; I < 2; I++) {
        memcpy(CurrPtr, Name[I], Len[I]);
        struct stat Stat;
        if (stat(File, &Stat)){
            return 0;
        }
        Time[I] = Stat.st_atimespec.tv_sec;
    }
    return Time[1]-(*Time);
}

Sorry for being obsessed with microoptimizations, but I'm looking to improve this.

S and SavesLen are both global variables defined and declared and initialized elsewhere. Is there a way to take advantage of local scopes to delete some of the memory earlier? Are there other improvements to be made? Thanks ahead of time.

An answerer pointed out that char File[SavesLen+Len[FirstLen > SecondLen]+2] is extremely hard to read. So here's why I do it that way. There are 2 files, and I need to check the access time of both of them. Rather than creating a path for each one, I'm reusing the same path, to minimize the number of VLA declarations and the number of memcpy() calls. Now let's break it down for you. char File[...] is a VLA declaration. The size is SavesLen plus the larger size of the 2 files, plus 2, for the / and null character. So what the heck is Len[FirstLen > SecondLen]? The reason why it's confusing is because I'm treating booleans like integers, and being weakly typed, C lets me do that. Len is an array so it will either be indexed by 0 or 1 (the 2 boolean values) depending on which one is bigger, because FirstLen > SecondLen will either evaluate to 0 or 1.

It seems the answerer understands, but in case there are others confused. I do agree that there was a bug, and that's because it used to be a ternary and I forgot to switch the values.

Answer 1

• One declaration per line, please.

• char File[SavesLen+Len[FirstLen > SecondLen]+2]; is extremely hard to read. In fact, I suspect that there is a bug. Correct me if I am wrong, an intention is to allocate enough memory to accomodate the longest pathname. Now, if FirstLen is indeed greater than SecondLen, the condition evaluates to true, which is in this context is 1. Now the VLA size becomes SaveLen + Len[1] + 2, that is SaveLen + SecondLen + 2, which is not enough for the longer first name.

• The loop running just two iterations is better be two calls to a function. Consider

    static time_t get_atime(const char * filename) {
        char path[SavesLen + 1 + strlen(filename) + 1];
        sprintf(path, "%s/%s", S, filename);
  
        struct stat stat; 
        stat(path, &stat);
        return stat.st_atimespec.tv_sec;
    }
  
    static int Q(const struct dirent **const First, const struct dirent **const Second) {
        time_t time0 = get_atime((*First)->d_name);
        time_t time1 = get_atime((*Second)->d_name);
        return time1 - time0;
    }
  

• Of course, Q is not a name for a function. compare_atime perhaps?