Add sponge(1)

[eadler]

Summary

This adds a naive implementation of sponge(1) to base.

sponge(1) is useful in pipelines to write to files read earlier without
setting up a temporary file inline

[eadler]

add default

[imp]

I like it. This looks great, but I'm concerned about performance for larger files since realloc copies data and can wind up with a fragmented heap that might artificially limit what you can do. Simple lists of ranges would suffice to avoid these copies without greatly increasing the complexity of the code.

usr.bin/sponge/sponge.1

49

It might be useful to specify what happens when memory can't be allocated for the entire file.

usr.bin/sponge/sponge.c

21

so if we fail to allocate, no output is produced. If so, this is OK.

89

why *4? This artificially may limit the file size as we approach the VM limits. *2 will result in more allocation but will let us get much closer to the limit without going nuts.

Also, this strategy is great for small files, but sucks for large files since we call realloc, which could be copying a crapton of data to keep the output code simple.

111

Why mix raw access and stdio? Seems like we could easily use all one or the other.

118

There's no harm in always closing the output, we're about to exit anyway.

[eadler]

Please wait for the next update and then review

usr.bin/sponge/sponge.c

111

because I was using printf for debugging and Just Worked. replacing with writev to avoid the reallocf problem you mentioned

118

true, but I dislike it, and it makes debugging annoying.

[eadler]

per @imp

[bcr]

Hi Eitan,

how about adding a small EXAMPLE section to the man page to show usage?

[danfe]

usr.bin/sponge/sponge.c

21

style(9) violation: should be static void *\nsafe_malloc.

28

return (ret);

31

Ditto.

38

Ditto.

48

Ditto.

52

Move { to the next line.

53

Usage is typically sent to stderr.

57

Ditto.

66

Local variables are utterly unsorted. See style(9).

[eadler]

Attempt style(9)

[eadler]

Fix history

[jilles]

An alternative implementation would write to a temporary file, then rename onto the destination once stdin closes. This is how ksh93's >;pathname redirection works (this does not mean I think this definitely should be done like that, or that the shell is the best place to implement this feature).

usr.bin/sponge/sponge.1

27

Convention seems to be not to add the leading zero here.

54

We should perhaps add a warning that the file will be written even if earlier components of the pipeline failed.

usr.bin/sponge/sponge.c

16

whitespace style: static void *safe_malloc...

78

style: spaces

80

-h for help is an unusual convention. It is very common for -h to mean something else, such as affecting a symlink instead of its target or making output more human-readable.

117

This memory is leaked (never used).

133

You can use err(3) here and elsewhere to simplify the code a bit.

137

If there are too many iovecs (sysconf(_SC_IOV_MAX), 16 MB with 16 KB buffers on FreeBSD), writev() will do nothing and return the error [EINVAL]. To avoid this, the write should be split up.

If sysconf(_SC_IOV_MAX) returns -1 or if you want to simplify the code, use the constant _XOPEN_IOV_MAX. This may lead to more system calls, but that may not be a problem.

144

The error from close() should be checked.

[danfe]

Some more style(9) issues...

usr.bin/sponge/sponge.c

20

Per our style(9), * should be part of the variable, not the type. Also applies to prototypes above.

23

Local variables are typically separated from the rest of of the function with extra linefeed.

29

Still not done.

35

Ditto.

41

Again, return (ret);.

44

Again, should be static void * (note the space).

47

Missing linefeed.

53

Again, return (ret);.

99

Per style(9), infinite loops are spelled as for (;;) in FreeBSD.

[eadler]

almost there...

usr.bin/sponge/sponge.1

54

Its the same for all unix commands, but its likely worth adding a reminder here.

usr.bin/sponge/sponge.c

80

It is annoying to me when utilities use -h for anything other than help.

137

this is ridiculous. Will fix in the next iteration.

[wblock]

usr.bin/sponge/sponge.1

40

Redundant.

utility reads standard in until complete, then opens
​the output file and writes to it.

41

I would say "useful" instead of "valuable".

42

This makes it useful in pipelines that read a file and then write to it.

43

s/The following/These/

46

s/Opens/Open/

51

s/In the event that/If/
and this needs a comma and some other stuff. I doubt you are referring to a specific malloc, but rather to memory allocation in general. So:

If an attempt to allocate memory fails,

54

s/will be/is/

54

Seems like this ought to be in a BUGS section at the end.

55

s/have//

67

This should use double quotes. Too bad we don't have a macro for air quotes, because that's pretty much what this is.

68

.Cm sort file | sponge file

[eadler]

handle too large bufs...

[eadler]

style(9)

[eadler]

usr.bin/sponge/sponge.1

54

this isn't a bug. Its statement on how shells work.

[eadler]

fix typo

[jilles]

usr.bin/sponge/sponge.1

54

missing space in iswritten

usr.bin/sponge/sponge.c

137

Ok, but to be portable you still need something like

if (maxiovec == -1)
    maxiovec =  _XOPEN_IOV_MAX;

since I expect haters of fixed limits to return -1 from sysconf(_SC_IOV_MAX).

[eadler]

last iteration - I hope!

[eadler]

add unit test

[eadler]

add unit test

[eadler]

fix copyright