[Asterisk-code-review] json.c/strings.c - Add a couple of utility functions (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/10981 )

Change subject: json.c/strings.c - Add a couple of utility functions
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/10981/1/main/strings.c
File main/strings.c:

https://gerrit.asterisk.org/#/c/10981/1/main/strings.c@253
PS1, Line 253: 	return -2;
> Returning -2 is truly weird and unexpected for a compare function. 
 > As a result you couldn't use it with qsort() and expect a sorted
 > array.  This should be 0 as both left and right strings are the
 > same (i.e., NULL).

It is odd I agree, but I wanted it to fail if a NULL pointer was set for one or both. At least for the use case I had, but the more expected/generic use case is probably to return 0. I toyed with that and adding a separate function. I guess I could do something like that.

 > 
 > Another question: Should NULL pointers be considered greater than
 > non-NULL pointers.  You currently sort NULL pointers before
 > non-NULL pointers.  It seems it would be more useful to sort NULL
 > pointers to the end of an array.

I used the reasoning that NULL is essentially less than anything, but sorting to the end makes sense, so I'll change.



-- 
To view, visit https://gerrit.asterisk.org/10981
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26f0a16d61537505eb41b4b05ef2e6d67fc2541b
Gerrit-Change-Number: 10981
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 08 Feb 2019 22:21:52 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190208/545d0be7/attachment.html>