On Fri, Nov 14, 2025 at 10:47:20AM +1100, raf <[email protected]> wrote: > On Wed, Nov 12, 2025 at 09:17:10PM -0500, Matt Panaro <[email protected]> wrote: > > > addresses https://savannah.gnu.org/bugs/index.php?66299 > > the ls program itself has both a parameter and a recognized environment > > variable, TIME_STYLE, that formats the output of the file's timestamp in > > the listing. To achieve partial parity for the -ls output of find, the > > code is refactored to honor the environment variable (a style parameter is > > not implemented herein). > > > > diff --git a/lib/listfile.c b/lib/listfile.c > > index e6ee70e3..78f89991 100644 > > --- a/lib/listfile.c > > +++ b/lib/listfile.c > > @@ -341,17 +341,27 @@ list_file (const char *name, > > /* Use strftime rather than ctime, because the former can produce > > locale-dependent names for the month (%b). > > > > + As ls does, honor the TIME_STYLE environment variable, if present; > > + otherwise: > > Output the year if the file is fairly old or in the future. > > POSIX says the cutoff is 6 months old; > > approximate this by 6*30 days. > > Allow a 1 hour slop factor for what is considered "the > > future", > > to allow for NFS server/client clock disagreement. */ > > - char const *fmt = > > - ((current_time - 6 * 30 * 24 * 60 * 60 <= statp->st_mtime > > - && statp->st_mtime <= current_time + 60 * 60) > > - ? "%b %e %H:%M" > > - : "%b %e %Y"); > > - > > + char *ts = getenv("TIME_STYLE"); > > + char const *fmt; > > + if(ts) { > > + /* ls expects TIME_STYLE to start with a '+'; if it were re-used > > + here, the '+' should be ignored when passing into strftime, > > + below. */ > > + if(*ts == '+') ts++; > > + fmt = ts; > > + } else { > > + fmt = ((current_time - 6 * 30 * 24 * 60 * 60 <= statp->st_mtime > > + && statp->st_mtime <= current_time + 60 * 60) > > + ? "%b %e %H:%M" > > + : "%b %e %Y"); > > + } > > while (!strftime (buf, bufsize, fmt, when_local)) > > buf = alloca (bufsize *= 2); > > > > -- > > 2.51.0 > > > > I think the use of alloca a should be changed to > malloc/realloc if taking in an environment variable. If > the length of $TIME_STYLE were a gigabyte, it would end > up with many calls to alloca with no cleanup of earlier > calls until the function ends, and so it could crash the > program when the stack filled up. Obviously, the > initial buffer size of 256 bytes was fine/unnecessary before, > and would still be fine for all real uses, but if you want > to be extra careful, it might be wise to replace: > > buf = alloca (bufsize *= 2); > > with something like: > { > if (buf == init_bigbuf) > buf = malloc(bufsize = strlen(fmt) * 4); > else if ((p = realloc(buf, bufsize * 2)) > buf = p, bufsize *= 2; > } > . > . > . > if (buf != init_bigbuf) > free(buf), buf = NULL; > > But that doesn't handle allocation failure, so it would > need more work. Maybe this: > > { > char *p; > if (buf == init_bigbuf) > buf = malloc (bufsize = strlen (fmt) * 2); > else if ((p = realloc (buf, bufsize * 2)) > buf = p, bufsize *= 2; > else /* realloc failed */ > free(buf), buf = NULL; > if (!buf) /* malloc or realloc failed */ > { > buf = init_bigbuf; > bufize = sizeof init_bigbuf; > fmt = ((current_time - 6 * 30 * 24 * 60 * 60 <= statp->st_mtime > && statp->st_mtime <= current_time + 60 * 60) > ? "%b %e %H:%M" > : "%b %e %Y"); > } > } > . > . > . > if (buf != init_bigbuf) > free(buf), buf = NULL; > > i.e., When faced with allocation failure, ignore $TIME_STYLE and > fall back to the original behaviour (which would never use more than > 256 bytes, so any further allocation wouldn't be necessary. > > If that all seems too silly, at least consider ignoring $TIME_STYLE > when the user is root. > > P.S. The new code in the patch uses tabs but the original code > uses only spaces. > > P.P.S. getenv takes a variable amount of time, depending on where > the variable appears in the list. It might be worthwhile to call > getenv (or secure_getenv) once at the start, or once when first > needed, and then remember its value for each use. It might not > make much difference (less than 2-100ns each time), but it seems > wasteful to repeat the same search through the list of environment > variables for every file being listed. > > cheers, > raf
P.P.P.S. Of course a much simpler solution is a sanity check on $TIME_STYLE to ignore it if it is longer than about 64 bytes. :-) Then the alloca doesn't need to change. cheers, raf
