Re: sendbug(1) patch

On 16/04/08 00:07 -0400, Ray wrote:
>You are requesting a change in sendbug because your IP address is
>blacklisted.  I think the proper solution to this problem does not
>involve sendbug at all.

This is how I currently use the sendbug:
1) sendbug
2) choose a)bort
3) vi /tmp/p.XXXXXX
4) remove '>SENDBUG' and other comments by myself
5) send it via msmtp

>
>>  That's different,  the temp file is not exactly the same as what is
>>  going to be sent to the pipe.
>
>sendbug sends a copy of the final report to the local user.

Not the same thing, see below.

>> What if there's a bug in the 'send_file' function?
>
>1) Are there any?
>2) Fix them.

Please check the patch below.

>3) We can't keep adding code (and bugs) for disaster scenarios.
>4) There are debuggers for this purpose.
>5) Your local user already gets a copy, whether it is by aborting or
>by sending the report.

Now I can see that...we are totally misunderstanding each other. It's my
fault not to point out the problem clearly.

In the 'send_file' function, I found it sends each line + '\0' + '\n' to
the pipe, (I think it's a bug), and the sendmail can happily accept it,
while other MTAs will report an error. So your idea about Cc: to a local
user will not reveal this problem, because the outputs to the pipe by
'sendbug' will be processed (masked) by sendmail. So the copy of local
user is not the same what 'sendbug' writes to the pipe.

Please note that I am not meaning adding this feature is in order to
only test the function, it's just an involuntary-side of this feature.

Originally, IMHO, sendbug will be better if only produce its raw PR mail
file for users, and let the users decide how to send it (use sendmail or
others), that is why I suggested the PR_AGENT feature.


--- /usr/src/usr.bin/sendbug/sendbug.c Fri Jan  4 08:50:09 2008
+++ sendbug.c Wed Apr 16 19:32:49 2008
@@ -315,10 +315,12 @@
  return (-1);
  case 0:
  close(filedes[1]);
- if (dup2(filedes[0], STDIN_FILENO) == -1) {
- warn("dup2 error: unsent report in %s",
-    pathname);
- return (-1);
+ if (filedes[0] != STDIN_FILENO) { /* defensive measure */
+ if (dup2(filedes[0], STDIN_FILENO) == -1) {
+ warn("dup2 error: unsent report in %s",
+ pathname);
+ return (-1);
+ }
  }
  close(filedes[0]);
  execl("/usr/sbin/sendmail", "sendmail",
@@ -330,6 +332,8 @@
  close(filedes[0]);
  /* Pipe into sendmail. */
  if (send_file(pathname, filedes[1]) == -1) {
+ wantcleanup = 0; /* without this,
+    temp won't be saved */
  warn("send_file error: unsent report in %s",
     pathname);
  return (-1);
@@ -467,7 +471,9 @@
  if (ep)
  copylen = sp - buf;
  else
- copylen = len;
+ /* The final '\n' has been stripped, so minus 1,
+ * otherwise the '\0' will be sent to pipe */
+ copylen = len - 1;
  if (atomicio(vwrite, dst, buf, copylen) != copylen ||
     atomicio(vwrite, dst, "\n", 1) != 1)
  goto end;

--
Dasn