Opened 4 years ago

Closed 4 years ago

#12609 closed bug (fixed)

/bin/mail doesn't handle piped in data

Reported by: AGMS Owned by: nobody
Priority: normal Milestone: Unscheduled
Component: Applications/Command Line Tools Version: R1/Development
Keywords: /bin/mail Cc: agmsmith@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

/bin/mail is now included in the Haiku image (thanks!) but doesn't work when piping the message text via standard input (the BeOS one does work that way too).

Needs an end-of-file detection as well as the manual period on a line test. If you want to get fancy, only use the "." end of message marker when reading from a terminal. gets() returns NULL at end of file, should test for that and break out of the line reading loop at that time. Example user level usage to test it (make sure the last line of the listing gets through too):

ls | mail -s Testing me@…

Attachments (3)

0001-Detect-terminal-input-end-of-file-parse-arguments-be.patch (4.9 KB ) - added by AGMS 4 years ago.
Patch to fix this ticket, and quite a few other bugs.
0001-Detect-terminal-input-end-of-file-better-argument-pa.patch (5.0 KB ) - added by AGMS 4 years ago.
Fixes several bugs in /bin/mail, now with coding style fixes.
0001-Detect-terminal-input-end-of-file-better-argument-pa.2.patch (5.4 KB ) - added by AGMS 4 years ago.
Now with stderr for certain messages (mostly stuff you need to see when stdout is sent to a file, stdout used for things you might want to save in a file), and style changes as suggested by Axel.

Download all attachments as: .zip

Change History (13)

comment:1 by diver, 4 years ago

Component: Applications/MailApplications/Command Line Tools
Owner: changed from czeidler to nobody

by AGMS, 4 years ago

Patch to fix this ticket, and quite a few other bugs.

comment:2 by AGMS, 4 years ago

Has a Patch: set

comment:3 by axeld, 4 years ago

Thanks! There are a number of minor (coding style) issues that you might want to fix to simplify accepting that patch:

  • We use sentence casing, ie. "No subject" over "No Subject"
  • No space after isatty
  • The printf in the following if-clause is a multi-line term, and must therefore be enclosed in curly braces
  • BString body automatically creates an empty string, so BString body = "" is superfluous
  • The comment after break should be on its own line like this:
    if (fgets(line, sizeof(line), stdin) == NULL) {
        // End of file or an error happened.
        break;
    }
    
    Extra points for actually checking for errors there, and reporting them :-)

comment:4 by AGMS, 4 years ago

Has a Patch: unset

by AGMS, 4 years ago

Fixes several bugs in /bin/mail, now with coding style fixes.

comment:5 by AGMS, 4 years ago

Has a Patch: set

comment:6 by korli, 4 years ago

Looks good. It still uses stdout to print error messages, wouldn't stderr be better?

comment:7 by AGMS, 4 years ago

Good idea, I'll update the patch today to use stderr for error messages and user prompts. Though I think verbose mode log messages should stay in stdout, since the user may want to capture them separately.

comment:8 by AGMS, 4 years ago

Has a Patch: unset

by AGMS, 4 years ago

Now with stderr for certain messages (mostly stuff you need to see when stdout is sent to a file, stdout used for things you might want to save in a file), and style changes as suggested by Axel.

comment:9 by AGMS, 4 years ago

Has a Patch: set

comment:10 by waddlesplash, 4 years ago

Resolution: fixed
Status: newclosed

Applied in hrev50117. Thanks!

Note: See TracTickets for help on using tickets.