Skip Menu |
 

This queue is for tickets about the MailTools CPAN distribution.

Report information
The Basics
Id: 36103
Status: resolved
Priority: 0/
Queue: MailTools

People
Owner: Nobody in particular
Requestors: EDAVIS [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 2.02
Fixed in: (no value)



Subject: Synopsis should show error checking
Download (untitled) / with headers
text/plain 707b
The example code in the Mail::Send pod documentation should check for errors, for example $fh = $msg->open or die "could not make message"; print $fh "Body of message" or die "could not write to message body"; $fh->close or die "could not send message"; After all, surely it is possible that sending the message fails. Even if print() to the filehandle cannot ever fail with the current Mail::Send implementation, that might not always be the case, and the normal specification of print() is that it returns false on failure. So I think the example code should encourage people to check for errors rather than ignoring them. (A small section in the doc on error handling would be good too.)
Download (untitled) / with headers
text/plain 445b
AFAIK, checking each print() does not add anything, because those errors will also be reported by close(). Not being the author of the code, I thing that I can deduct that the Mail::Send->open (is Mail::Mailer->new()) dies on errors. So also here you do not need to check the return code. If you would update the man-page, it probably should be: my $s = eval { Mail::Send->new }; die if $@; print $s "text\n"; close $s or die; Right?
Download (untitled) / with headers
text/plain 254b
Show quoted text
>AFAIK, checking each print() does not add anything, because those >errors will also be reported by close().
Fair enough - in this case there needs to be an explicit statement in the doc that you don't need to check print() but you should check close().
Subject: Re: [rt.cpan.org #36103] Synopsis should show error checking
Date: Fri, 23 May 2008 20:39:10 +0200
To: EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <mark [...] overmeer.net>
Download (untitled) / with headers
text/plain 980b
* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080523 18:13]: Show quoted text
> Queue: MailTools > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36103 > >
> >AFAIK, checking each print() does not add anything, because those > >errors will also be reported by close().
> > Fair enough - in this case there needs to be an explicit statement in > the doc that you don't need to check print() but you should check close().
It seems fair to only describe what people should do (check close() in Mail::Send and Mail::Mailer), and to the describe what people do not have to do. So, I leave that part to "perldoc -f print" -- which does not explain it ;-b -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Download (untitled) / with headers
text/plain 491b
perlfunc(1): Show quoted text
> print Prints a string or a list of strings. Returns true if > successful.
This implies that you should always check the return value of print, since it could fail and return false. (Unless you really don't care whether it succeeds or not.) So if this is not the case for Mail::Send, you do need to say so. However, you leave yourself more wiggle room for future expansion if you document that print() can fail, and give example code that checks it.
Subject: Re: [rt.cpan.org #36103] Synopsis should show error checking
Date: Tue, 27 May 2008 11:06:08 +0200
To: EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <solutions [...] overmeer.net>
Download (untitled) / with headers
text/plain 1.9k
* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080527 08:07]: Show quoted text
> Queue: MailTools > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36103 > > > perlfunc(1): >
> > print Prints a string or a list of strings. Returns true if > > successful.
> > This implies that you should always check the return value of print, > since it could fail and return false. (Unless you really don't care > whether it succeeds or not.)
This return status indicates an error when the OS has signalled the program that something is wrong with the file-handle. It does not reflect whether the actual characters were written. Characters are written into the output buffers, which are within your program, until a buffer of 4 kB is full. Then, the stdio will transfer it to kernel space. If something goes wrong there, the last print will probably return an error. However, once in the kernel, it is still not sure that the characters will end-up on disk. The manual page of close explains it NOTES Not checking the return value of close is a common but nevertheless serious programming error. It is quite possible that errors on a pre‐ vious write(2) operation are first reported at the final close. Not checking the return value when closing the file may lead to silent loss of data. This can especially be observed with NFS and with disk quota. So: I suspect that checking the return value of print is giving you unjustified feeling of additional protection. It doesn't help much, and makes programs overly complicated. The best sign that something is wrong you probably get from Mail::Send/Mail::Mailer is a SIGPIPE from the OS. Don't ignore SIGPIPE. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Download (untitled) / with headers
text/plain 261b
Thanks for the explanation. Unless $| is on, you are surely right that checking close() is important but checking print() is less so. (All that is needed is a guarantee that if there were any errors writing to the file earlier on, close() will pick them up.)
Subject: Re: [rt.cpan.org #36103] Synopsis should show error checking
Date: Thu, 29 May 2008 13:52:32 +0200
To: EDAVIS via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <solutions [...] overmeer.net>
Download (untitled) / with headers
text/plain 875b
* EDAVIS via RT (bug-MailTools@rt.cpan.org) [080529 11:05]: Show quoted text
> Thanks for the explanation. Unless $| is on, you are surely right that > checking close() is important but checking print() is less so.
Even when $|==1, the return print() will only show a very limited number of possible problems. If you use fsync(), then you will get more errors immediately. In either case, performance suffers tremendously. Both $| and fsync() should be used a little as possible. Getting speedy return codes with print is not a valid reason (in 99.9% of the cases). -- MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
no use


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.