127 lines
4.7 KiB
Plaintext
127 lines
4.7 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "d61334b6_ee7f2cdf",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 1000074
|
|
},
|
|
"writtenOn": "2023-09-18T08:37:56Z",
|
|
"side": 1,
|
|
"message": "Once s/del/free/, +1 from me.",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "a368c5f7_b860a266",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 17,
|
|
"author": {
|
|
"id": 1000074
|
|
},
|
|
"writtenOn": "2023-09-18T08:37:56Z",
|
|
"side": 1,
|
|
"message": "we usually call these free as a counterpart of alloc.\nThis also makes it clear by convention that passing NULL to it is allowed as a NOOP.",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "d19bf164_7039b95a",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 17,
|
|
"author": {
|
|
"id": 1000010
|
|
},
|
|
"writtenOn": "2023-09-18T08:45:45Z",
|
|
"side": 1,
|
|
"message": "Well, this \"usual\" naming does not apply here:\n\n```\ninclude/osmocom/hlr/gsup_router.h:int gsup_route_del_conn(struct osmo_gsup_conn *conn);\ninclude/osmocom/hlr/hlr_ussd.h:void ussd_route_del(struct hlr_ussd_route *rt);\n```\n\nThis patch is consistent with the existing code, and IMO we cannot require this from contributors if we ourselves don\u0027t always follow such requirements it in the existing code...",
|
|
"parentUuid": "a368c5f7_b860a266",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "185a6aa4_a806beb9",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 17,
|
|
"author": {
|
|
"id": 1000230
|
|
},
|
|
"writtenOn": "2023-09-18T13:59:05Z",
|
|
"side": 1,
|
|
"message": "I modeled this code after the existing one for EUSEs and USSD routes, which - as @fixeria noted - uses _del rather than _free. What is the community consensus here? Should I change my patch to use _free? Should all existing usage of _del in osmo-hlr also be changed to _free?",
|
|
"parentUuid": "d19bf164_7039b95a",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "c3f853fa_dd6d58db",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 17,
|
|
"author": {
|
|
"id": 1000074
|
|
},
|
|
"writtenOn": "2023-09-18T15:13:01Z",
|
|
"side": 1,
|
|
"message": "My opinion is that in 88% of the code we use alloc+free, so in general those named _del should be moved to _free (as long as they can be considered free function as per what they do).\n\nSemantics of free functions for object pointers are clear. They get a nullable heap allocated pointer and free it together with all fields/subobjects.",
|
|
"parentUuid": "185a6aa4_a806beb9",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "c974108c_a1eacc3b",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 17,
|
|
"author": {
|
|
"id": 1000230
|
|
},
|
|
"writtenOn": "2023-09-18T18:29:54Z",
|
|
"side": 1,
|
|
"message": "The existing euse_del() and ussd_route_del() functions don\u0027t have those \"free\" semantics you just stated: instead they require the pointer to be valid (not NULL) and they perform the llist_del() step (unlinking from the global lists of EUSEs and USSD routes) before the final talloc_free(). My new smsc_del() and smsc_route_del() functions are exactly the same.",
|
|
"parentUuid": "c3f853fa_dd6d58db",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "085ddeba_cacfda19",
|
|
"filename": "include/osmocom/hlr/hlr_sms.h",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 29,
|
|
"author": {
|
|
"id": 1000074
|
|
},
|
|
"writtenOn": "2023-09-18T08:37:56Z",
|
|
"side": 1,
|
|
"message": "free",
|
|
"revId": "da05bc066ebc94eda14dfcf364a30e50211010bc",
|
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
|
}
|
|
]
|
|
} |