Eric Blake wrote: > Jim Meyering <jim <at> meyering.net> writes: > >> > mktemp a => error, no run of X >> > mktemp aXX => error, run of X is too short >> > mktemp XXX => generates 3-character name (if possible) >> > mktemp aXXX.b => generates 6-character name (if possible) >> > mktemp --suffix=.b aXXX => longer spelling of the above line >> > mktemp aXXX --suffix=.b => likewise, if not POSIXLY_CORRECT >> > mktemp aXXXbXXX => generates aXXXb123 (only the last run changed) >> > mktemp --suffix=X aXXX => only way to generate a123X instead of a1234 >> > mktemp --suffix=.txt file => error, no run of X >> > mktemp --suffix=.txt aXXXb => error, explicit --suffix requires trailing X >> >> Nice, comprehensive set of examples. > > Here's my first cut at implementing this. > > Eric Blake (4): > [1/4] build: update gnulib submodule to latest, for tempname changes > Real gnulib commit id TBD, once I push my pending mkostemps series to gnulib > (http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/19282). Note that either > patch 1 or patch 2 in isolation will cause bootstrap failure; so even though > you normally like to do gnulib submodule updates in isolation, I'm wondering > if > this is a case where the two patches should be squashed together.
This sounds like a good reason indeed to include the gnulib update with the dependent change. FWIW, I'm less averse to including a gnulib submodule update with other changes, now that I've become more used to dealing with submodules. > [2/4] build: reflect gnulib changes to tempname > Since gnulib added mkostemps support to gen_tempname, we need to reflect the > extra parameter in gen_tempname_len. I think the .diff format makes it easier > to see how we intentionally differ from glibc, rather than writing a flat-out > replacement file (it's also more robust at catching subsequent gnulib changes > to the same file, to let us know to resynchronize). Yes, I was thinking the same thing. This would be easier to review if there were two deltas: - the no-semantic-change, convert-to-.diff one - the add-argument semantics-changing one but it's not that big a deal. Yeah, it's a shame about glibc using the incorrect type for the new parameter. > [3/4] tests: enhance mktemp test > Add more tests of existing behavior, to ensure I didn't break it, and to make > it obvious what I'm changing. I could rebase the series to put this first, > alongside my pending mktemp doc patch, especially if your review of my doc > patch and the accompanying email turn up any changes we want to existing > behavior. Those new tests look fine. > [4/4] mktemp: add suffix handling > The real fun. Hopefully I added enough tests to cover everything new that > results from the new code. Also, is the rearranged output in usage() okay, > and > should I split that into a separate patch? I'll probably run out of time here, but will start commenting. AUTHORS: Glad you added your name. > In rereading this before hitting the send button, I see an out-of-bounds > reference in patch 4. I'll know if you reviewed this if you spot the same > bug ;) Also, I guess I should also amend my series to mention this bug report > in the commit message: >> > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316 Better to stop now. Will resume here in the morning and then revisit your doc-change message.
