[asterisk-dev] [Code Review] fix excess warnings after r1395 (and disallow empty names always)

[wdoekes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1560/
-----------------------------------------------------------

(Updated Nov. 2, 2011, 4:57 p.m.)


Review request for Asterisk Developers, Terry Wilson and schmidts.


Changes
-------

Thanks for the reviews.

Is this more to your liking Stefan?


Summary
-------

r1395 broke two things:
- It added warnings for situations that shouldn't get a warning. (Bad placement of the warning.) [schmidts found this]
- It was allowed to have an empty sippeer name. (No check after get_name_from_variable().)

However, both before and after r1395, the following was broken:
- For port-insecure matches, it was allowed to have an empty peer name. (Because of the use of ast_load_realtime_multientry.)
- Realtime lookups for name NULL were attempted with undefined behaviour (res_config_sqlite returns a warning "1 parameter and 1 value at least required" and no results)

This patch does the following:
- Fix the warning to only be emitted when there actually is an empty peername found. (Both in case of sipregs and sippeers.)
- Disallow the empty peername always. Even for port-insecure matches.

Notes:
- For sippeer matching: if name is empty, the match is used and discarded at the end with a warning. (The behaviour as before r1395.)
- For the port-insecure matching of sipregs: if name is empty, the match is discarded and lookup continues. (This wasn't available before r1395.)
- For other matching of sipregs: if name is empty, a warning is emitted, the match is discarded and lookup continues. (This was undefined before r1395; the NULL-name was refused, the empty-name was allowed.)


Diffs (updated)
-----

  /branches/1.8/channels/chan_sip.c 343158 

Diff: https://reviewboard.asterisk.org/r/1560/diff


Testing
-------

I've been staring at this for a couple of hours now. It should be better than it was.


Thanks,

wdoekes

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111102/e11ab434/attachment-0001.htm>