Re: [PATCH RFC 1/4] misc-utils: add the pipesz utility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for your review, Karel! I appreciate all of your feedback.

> Would be better to be paranoid (as expected by --check) by default and implement --force for cases when user wants something crazy?

The manpage talks a bit about this. pipesz is designed to be opportunistic by default in order to avoid annoying users that are troubleshooting their pipelines.

For example, when I'm working on a script, I'll often need to cut it up into parts and dump intermediate data to the terminal or to a file. So, if I start with a script like "a | pipesz b | c | d" and realize that there's some problem with "b" I might end up running "a | pipesz b" at my terminal or commenting out the "| c | d". If pipesz weren't lenient, it'd spit out an error because stdout is now a tty. So now I've got two annoyances, "b" is broken *and*​ pipesz is whining. Incidentally, this is how pipesz was originally. I did not like it.

There's another more subtle reason, too. I don't think that failure to resize a pipe is a reason to make scripts suddenly stop working. If your box is low on memory or you're near pipe-user-pages-soft, you'd probably want to ignore the fact you can't get 1 meg for a pipe right now and just have your script run. If you really, really care that you've got your pipes and ttys and files and sockets and resource limits straight, that's what --check is for. I think that forgetting to specify --force is a footgun just waiting to take down an overnight batch job.

If you don't find this argument persuasive, I'll switch the sense of --check/-c to --ignore/-t (like ionice, since -i is taken), as requested. I find --force misleading, but I recognize it is long-standing convention and will do this instead of --ignore/-t if you insist.

> Why we need multiple getopt_long() blocks?

I had a feeling this would come up. I should have talked about this in the cover letter. I had a single argument processing loop originally, but in the end I felt that it was not worth the trade-offs involved.

In my mind there were 5 implementation strategies I could choose:

(1) Require -c/-g/-q/-v to precede other arguments, so we have all the information we need to process the other options immediately. This is not viable for a number of reasons that should become apparent quickly.
(2) Squirrel away -f/-n arguments into a liked list or xrealloc'ing arrays. This adds manual memory management, bookkeeping to keep valgrind happy on exit(), and an OOM failure case. I chose a list, and it ended up being about 1/3 of the total code using "list.h" and "xalloc.h".
(3) Squirrel away -f/-n arguments in fixed-sized arrays. This introduces a very small amount of bookkeeping, but also a bizarre failure mode should someone try to have too much fun with, e.g., xargs. Also, it would end up changing the order of operations unless done in a more complicated way.
(4) Only allow a single -f/-n argument (perhaps in addition to -i/-o/-e). I believe this is what you proposed. This could lead to some annoying repetitiveness in some cases. Additionally, it would complicate documentation if we permitted combining -f/-n with -i/-o/-e. Or lead to even more stuttering if we went for maximum simplicity and took only one of -f/-n/-i/-o/-e.
(5) Scan the argument list multiple times. The downside here is that's its unconventional. Heretical, even!

You caught on to this, but options 1-4 also annoy people like me who respond to invocation errors with "[up arrow] --help" instead of using readline shortcuts or shell history expansion.

Of the options above, excepting 5 of course, I lean heavily towards 4.

> The rest seems good.

Thank you! Let me know what changes you'd like to see and I'll whip up a v2!

Best wishes,
    - Nathan Sharp




[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux