[Home]

Summary:ASTERISK-01536: [patch] utils.h -- ast_strlen_zero() potential crash protection
Reporter:Rob Gagnon (rgagnon)Labels:
Date Opened:2004-05-04 11:28:32Date Closed:2011-06-07 14:10:24
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) utils.h.patch.txt
Description:Someone forgot to check s for NULL.  Attached tiny patch fixes that
Comments:By: Mark Spencer (markster) 2004-05-04 11:58:07

strlen does not check for NULL either, therefore this is not necessary.
By: Mark Spencer (markster) 2004-05-04 12:22:52

This isn't a bug, but if you see anyone who actually is using ast_strlen_zero in a way that could cause a crash, that *would* be a bug.
By: Rob Gagnon (rgagnon) 2004-05-04 12:47:58

We have no control over the guts of strlen().... Does that mean that a wrapper of it should not change the functionality?

Does everyone believe in blaming the caller of the function if they use the function wrong?

Is there something wrong with writing a function that protects itself from a crash?

Changing the return value in ast_strlen_zero() could actually be as simple as:

return(!s || (*s == '\0'));  

now that I looked at it better, so why not just prevent a crash globally by changing this one line?

If asterisk crashes because someone in another place calls ast_strlen_zero() with a NULL ptr, you are right---the bug is theirs, but the perception will be that Asterisk crashed.  Do you want that PR?

Also..... What other way is there to submit a "tweak" without it coming out as a "bug".... the only option on the menu is "Report Bug"

edited on: 05-04-04 11:46

edited on: 05-04-04 11:47
By: James Golovich (jamesgolovich) 2004-05-04 14:19:26

I'm with Mark on this one for sure.  Thats exactly why it was implemented this way.  It's much better to leave it to the caller to check if the pointer is valid.  There are many cases where something else is checking if the pointer is valid and if we did it in ast_strlen_zero it would be a few wasted cycles
By: Mark Spencer (markster) 2004-05-04 14:24:17

It doesn't mean we're stuck to duplicate limitations in the original, but it does mean we have to make judgement calls about when they do and don't make sense.  In this case, strlen is called *very* frequently and is *always* already checked for NULLness in any place it could possibly be NULL.  I do not want to increase the overhead to again check for NULL, especially knowing that the code has already been done this way.  

If you want to add another utility called strlen_zero_or_null() and replace occurances of if (foo && strlen(foo)) with if (!strlen_zero_or_null(foo)) then that's fine too.
By: James Golovich (jamesgolovich) 2004-05-04 14:38:33

case closed.  If someone really wants something like strlen_zero_or_null() implemented, reopen this case