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.

• pacmaninbw: Welcome to the Code Review Community. It would be very helpful to use if you explained what the program is trying to do, and to provide missing pieces of the program such as the definition of the struct dirent. Have you profiled your code to know that his is actually a bottleneck?
• vnp: @pacmaninbw The purpose of the code is made clear by mentioning scandir. struct dirent is defined in <dirent.h>. I don't think it has to be spelled out here.

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?

• user235747: So you're suggesting to abstract away the process of getting access time? Got it
• user235747: Edited my question to reflect this answer