Re: Unchecked sprintf/strcat?
On 2006-05-29 21:51:13 +0900, TAKAHASHI Tamotsu wrote:
> From: TAKAHASHI Tamotsu <ttakah@xxxxxxxxxxxxxxxxx>
> Because we have no way to detect overflow after safe_strcat,
> I make safe_strcat return NULL when truncation occurs.
Eeeeek. Don't do that.
Return an int like in snprintf if all you want to do is know
more about how much safe_strfcat copied. If you want to be
able to use the result of safe_strfcat in other string
operations, return a pointer to the destination string.
But please don't return a pointer that happens to be NULL under
some non-fatal error condition. That's as useless as a return
value can get.
> snprintf returns "the number of characters that would have been
> output if the size were unlimited." - according to "man 3 snprintf."
> So it's easy to detect overflow.
>
> strncpy can't do a good job. I just check strlen before strfcpy.
> (Well, I haven't tried
> "#define safe_strfcpy(A,B,C) *(A)=0;safe_strcat(A,C,B)")
Eeeeek again.
You basically doubled the runtime of safe_strfcpy. Also, the
macro you did there isn't safe with respect to side effects.
Don't do that.
Also, please don't bother users with mutt_error output about
truncated strings. When something goes wrong and indicates
mutt gets into an undefined state, abort(). When the user
needs to take action, do mutt_error().
dprint may be appropriate in the places you use here, but I'm
not at all convinced it's worth the effort.
Frankly, if you want to improve string handling in mutt, move
it away from fixed-size buffers completely. That's a better
use of your time than trying to add error messages where
fixed-size buffers are still used.
Regards,
--
Thomas Roessler · Personal soap box at <http://log.does-not-exist.org/>.