Hoi. [2020-03-19 12:45] Philipp Takacs <phil...@bureaucracy.de> > > I have also looked at folder_read and found it't a bit ugly ;-)
A good indicator for starting to rework. :-) > I have rewritten it with scandir. This saves about 40 loc and improves > the reading flow for this funktion. Sounds good! Some comments: > Also noticed the other flag, not shoure if we need this realy. It's > only used in folder and I don't see the point of knowing that there > are other files in a folder. We should be a bit careful at that point to understand the situation. Here are some aspects to think about: - Files starting with a comma. These were backups of deleted messages (see the section on the trash folder in my master's thesis). Thus they are considered MH files here but not further regarded. I'm not sure if the code had been this way all the time, because there used to the a `--with-hash-prefix' option (or similar) that replaced the comma with a hash symbol as the prefix for deleted messages backups. Anyways, we don't have such comma files anymore. Mmh uses the trash folder for that case, i.e. we can get rid of the comma prefix file handling ... possibly. Does anyone know the situation in nmh: have they switched to a trash folder as well? Because we're still (basically) mail storage compatible to nmh (I think), we should not be fully ignorant of that situation. But we could handle those backup files as `other' files. - `Other' files seem to be files in the mail storage that have nothing to do with MH. E.g. any random file someone created in the mail storage without any connection to mmh. Normally mmh would simply ignore them, i.e. act as if they wouldn't be there at all. There's one place, however, where `other' files are relevant: rmf(1). We should not delete a mail folder that includes `other' files. It seems this check is missing there. Hence, I would keep the `other' files marker. It adds few complexity and can be useful. Plus, we should use it in rmf(1). > +static int others; > + > +static int > +msgcmp(const struct dirent **d1, const struct dirent **d2) > +{ > + size_t l1, l2; > + > + l1 = strlen((*d1)->d_name); > + l2 = strlen((*d2)->d_name); > + if (l1 < l2) { > + return -1; > + } > + if (l1 > l2) { > + return 1; > + } > + return strcmp((*d1)->d_name, (*d2)->d_name); > +} Isn't that a strange comparison algorithm, for first compare string lengths and only if eaqual their content? Here does mmh sort by length of folder name? > +static int > +msgfilter(const struct dirent *e) > +{ > + int i; > + if (e->d_name[0] == '.' || e->d_name[0] == ',') { About this comma: see above! > + return 0; > + } > + for (i = 0; e->d_name[i]; i++) { > + //if ((i > 1 && e->d_name[i] < '0') || e->d_name[i] < '1' || > e->d_name[i] > '9') { C99-style comment ... looks to be not finished at this point. > + if (e->d_name[i] < '0' || e->d_name[i] > '9') { > + others = 1; > + return 0; > + } > + } > + return 1; > +} Wow, it feels so good to have some touch with the mmh development again! I'm really looking forward to dig into the code as well. Hoping I get that managed soon. meillo