Re: intended MBF: wrong redirections in maintainer scripts
On Thu, Aug 09, 2018 at 11:03:22PM +0200, Wouter Verhelst wrote: > On Thu, Aug 09, 2018 at 09:48:22PM +0200, Ralf Treinen wrote: > > You are absolutely right that our assumption about the authors intention > > may be wrong, and that they really intended the redirection the way they > > wrote it. This means that we should be more careful, and ignore cases in > > which the above pattern occurs in a context which has itself a redirection. > > As a consequence, we wouldn't detect a bug in a case like this one: > > > > (foo 2>&1 1> /dev/null) | /some/processing > > > > This should be enough to eliminate false positives, right? > > Yes, I think so; or more generally, where the stderr output is caught > futher on. E.g., something like this wouldn't be a bug either: > > ERRORS=$(foo 2>&1 1>/dev/null) > > But again, I do agree that in general, the assumption that the ordering > of redirections was mistaken is probably the right one. Indeed, we also have to check for this case, and probably look over all detected cases to be sure. Thanks for your constructive remarks. We will propbably do the MBF at the end of the moth, after return from [VAC]. -Ralf.
Re: intended MBF: wrong redirections in maintainer scripts
On Thu, Aug 09, 2018 at 09:48:22PM +0200, Ralf Treinen wrote: > Hello Wouter, > > On Tue, Aug 07, 2018 at 12:38:32PM +0200, Wouter Verhelst wrote: > > On Sat, Aug 04, 2018 at 01:15:57PM +0800, Ralf Treinen wrote: > > > Hi, > > > > > > as announced in our talk at debconf'18 [1] we intend a MBF about wrong > > > redirections in maintainer scripts. In general these are of the form > > > > > > foo 2>&1 1> /dev/null > > > > > > Here it was probably intended to send both stderr and stdout to /dev/null. > > > > What makes you say that? ;-) > > > > It may be that the maintainer did indeed want stdout to be discarded, > > but stderr not; for instance because they wanted to parse the stderr > > output. > > > > (not saying this is the most likely case, but you might want to > > double-check that before filing the bugs) > > We were assuming that the author of the script wanted to send both 1 and 2 > to /dev/null and was victim to the common mistake of getting the order of > redirections wrong. Right. In general, I do agree that this is the more likely of possibilities. The required ordering of redirections is just confusing, tbh. > You are absolutely right that our assumption about the authors intention > may be wrong, and that they really intended the redirection the way they > wrote it. This means that we should be more careful, and ignore cases in > which the above pattern occurs in a context which has itself a redirection. > As a consequence, we wouldn't detect a bug in a case like this one: > > (foo 2>&1 1> /dev/null) | /some/processing > > This should be enough to eliminate false positives, right? Yes, I think so; or more generally, where the stderr output is caught futher on. E.g., something like this wouldn't be a bug either: ERRORS=$(foo 2>&1 1>/dev/null) But again, I do agree that in general, the assumption that the ordering of redirections was mistaken is probably the right one. -- Could you people please use IRC like normal people?!? -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008 Hacklab
Re: intended MBF: wrong redirections in maintainer scripts
Hello Wouter, On Tue, Aug 07, 2018 at 12:38:32PM +0200, Wouter Verhelst wrote: > On Sat, Aug 04, 2018 at 01:15:57PM +0800, Ralf Treinen wrote: > > Hi, > > > > as announced in our talk at debconf'18 [1] we intend a MBF about wrong > > redirections in maintainer scripts. In general these are of the form > > > > foo 2>&1 1> /dev/null > > > > Here it was probably intended to send both stderr and stdout to /dev/null. > > What makes you say that? ;-) > > It may be that the maintainer did indeed want stdout to be discarded, > but stderr not; for instance because they wanted to parse the stderr > output. > > (not saying this is the most likely case, but you might want to > double-check that before filing the bugs) We were assuming that the author of the script wanted to send both 1 and 2 to /dev/null and was victim to the common mistake of getting the order of redirections wrong. You are absolutely right that our assumption about the authors intention may be wrong, and that they really intended the redirection the way they wrote it. This means that we should be more careful, and ignore cases in which the above pattern occurs in a context which has itself a redirection. As a consequence, we wouldn't detect a bug in a case like this one: (foo 2>&1 1> /dev/null) | /some/processing This should be enough to eliminate false positives, right? -Ralf.
Re: intended MBF: wrong redirections in maintainer scripts
Adam Borowski wrote: > On Tue, Aug 07, 2018 at 12:38:32PM +0200, Wouter Verhelst wrote: >> On Sat, Aug 04, 2018 at 01:15:57PM +0800, Ralf Treinen wrote: >> > as announced in our talk at debconf'18 [1] we intend a MBF about wrong >> > redirections in maintainer scripts. In general these are of the form >> > >> > foo 2>&1 1> /dev/null >> > >> > Here it was probably intended to send both stderr and stdout to >> > /dev/null. >> >> What makes you say that? ;-) >> >> It may be that the maintainer did indeed want stdout to be discarded, >> but stderr not; for instance because they wanted to parse the stderr >> output. >> >> (not saying this is the most likely case, but you might want to >> double-check that before filing the bugs) > > Oy vey... I didn't notice this when Ralf's mail was posted (merely > checked whether I'm or QA are on the dd-list). But, indeed, this whole > MBF is wrong. Thanks Wouter! Not wrong, just having the potential for false positives. A cursory inspection using codesearch showed me 43 examples that where the redirections are wrong and only 1 example where the script was actually trying to capture stderr. (The code used by Ralf to find these picks up many more examples than my simple grep will pick up.) It may also be that Ralf and his team are already filtering out places where the output is captured; the talk by Ralf and Nicholas at DebConf is well worth watching. https://debconf18.debconf.org/talks/90-mining-debian-maintainer-scripts/ Not restricting the search to maintainer scripts finds many many more... it's a common enough mistake. regards Stuart -- Stuart Prescotthttp://www.nanonanonano.net/ stu...@nanonanonano.net Debian Developer http://www.debian.org/ stu...@debian.org GPG fingerprint90E2 D2C1 AD14 6A1B 7EBB 891D BBC1 7EBB 1396 F2F7
Re: intended MBF: wrong redirections in maintainer scripts
On Tue, Aug 07, 2018 at 12:38:32PM +0200, Wouter Verhelst wrote: > On Sat, Aug 04, 2018 at 01:15:57PM +0800, Ralf Treinen wrote: > > as announced in our talk at debconf'18 [1] we intend a MBF about wrong > > redirections in maintainer scripts. In general these are of the form > > > > foo 2>&1 1> /dev/null > > > > Here it was probably intended to send both stderr and stdout to /dev/null. > > What makes you say that? ;-) > > It may be that the maintainer did indeed want stdout to be discarded, > but stderr not; for instance because they wanted to parse the stderr > output. > > (not saying this is the most likely case, but you might want to > double-check that before filing the bugs) Oy vey... I didn't notice this when Ralf's mail was posted (merely checked whether I'm or QA are on the dd-list). But, indeed, this whole MBF is wrong. Thanks Wouter! The rarer case of silencing both stdout and stderr tends to be written: foo >/dev/null 2>/dev/null -- or at least I've been taught so, as this doesn't look like the common case. Ie: foo 2>&1 /dev/null which you somehow have a problem with. Don't you use this very construct every a few days? Meow! -- ⢀⣴⠾⠻⢶⣦⠀ So a Hungarian gypsy mountainman, lumberjack by day job, ⣾⠁⢰⠒⠀⣿⡁ brigand by, uhm, hobby, invented a dish: goulash on potato ⢿⡄⠘⠷⠚⠋⠀ pancakes. Then the Polish couldn't decide which of his ⠈⠳⣄ adjectives to use for the dish's name.
Re: intended MBF: wrong redirections in maintainer scripts
On Sat, Aug 04, 2018 at 01:15:57PM +0800, Ralf Treinen wrote: > Hi, > > as announced in our talk at debconf'18 [1] we intend a MBF about wrong > redirections in maintainer scripts. In general these are of the form > > foo 2>&1 1> /dev/null > > Here it was probably intended to send both stderr and stdout to /dev/null. What makes you say that? ;-) It may be that the maintainer did indeed want stdout to be discarded, but stderr not; for instance because they wanted to parse the stderr output. (not saying this is the most likely case, but you might want to double-check that before filing the bugs) -- Could you people please use IRC like normal people?!? -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008 Hacklab
Re: intended MBF: wrong redirections in maintainer scripts
Hi, > as announced in our talk at debconf'18 [1] we intend a MBF about wrong > redirections in maintainer scripts. In general these are of the form > > foo 2>&1 1> /dev/null > > Here it was probably intended to send both stderr and stdout to /dev/null. > In reality the effect of this is to send only 1 to /dev/null, and to send > 2 to the file that 1 was sent to before that line. when you file bugs, please also include a correct example. Looking at your talk this seems to be: foo >/dev/null 2>&1 Thanks, Carsten
