On Tue, Apr 18, 2017 at 06:07:43PM +0200, René Scharfe wrote: > > - fscanf(fp, "%"SCNuMAX" %127c", &pid, locking_host) == 2 && > > + fscanf(fp, scan_fmt, &pid, locking_host) == 2 && > > /* be gentle to concurrent "gc" on remote hosts */ > > (strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM); > > if (fp != NULL) > > > > How important is it to scan the whole file in one call? We could split > it up like this and use a strbuf to handle host names of any length. We > need to be permissive here to allow machines with different values for > HOST_NAME_MAX to work with the same file on a network file system, so > this would have to be the first patch, right? I doubt that doing it in one call matters. It's not like stdio promises us any atomicity in the first place. > - fscanf(fp, "%"SCNuMAX" %127c", &pid, locking_host) == 2 && > + fscanf(fp, "%"SCNuMAX" ", &pid) == 1 && > + !strbuf_getwholeline(&locking_host, fp, '\0') && I don't think there is anything wrong with using fscanf here, but it has enough pitfalls in general that I don't really like its use as a parser (and the general lack of it in Git's code base seems to agree). I wonder if this should just read a line (or the whole file) into a strbuf and parse it there. That would better match our usual style, I think. I can live with it either way. > NB: That && cascade has enough meat for a whole function. Yeah. -Peff