Opened 3 years ago

Closed 3 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 3 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 3 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 3 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 Changed 3 years ago by diver

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

Changed 3 years ago by AGMS

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

comment:2 Changed 3 years ago by AGMS

Has a Patch: set

comment:3 Changed 3 years ago by axeld

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 Changed 3 years ago by AGMS

Has a Patch: unset

Changed 3 years ago by AGMS

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

comment:5 Changed 3 years ago by AGMS

Has a Patch: set

comment:6 Changed 3 years ago by korli

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

comment:7 Changed 3 years ago by AGMS

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 Changed 3 years ago by AGMS

Has a Patch: unset

Changed 3 years ago by AGMS

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 Changed 3 years ago by AGMS

Has a Patch: set

comment:10 Changed 3 years ago by waddlesplash

Resolution: fixed
Status: newclosed

Applied in hrev50117. Thanks!

Note: See TracTickets for help on using tickets.