Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13121 )
Change subject: add osmo_use_count API ...................................................................... Patch Set 5: (1 comment) https://gerrit.osmocom.org/#/c/13121/5/include/osmocom/core/use_count.h File include/osmocom/core/use_count.h: https://gerrit.osmocom.org/#/c/13121/5/include/osmocom/core/use_count.h@154 PS5, Line 154: * foo_get(foo, "bar"); // one osmo_use_count_entry was allocated : * foo_get(foo, "baz"); // a second osmo_use_count_entry was allocated : * foo_get(foo, "baz"); // still two entries : * : * printf("use: %s\n", osmo_use_count_name_buf(namebuf, sizeof(namebuf), &foo->use_count)); : * // "use: 3 (bar,2*baz)" : * : * foo_put(foo, "bar"); // still two entries, one entry is idle ("bar"=0) : * foo_put(foo, "baz"); I know this is just an example, but at least in any code that eveer uses this I strongly suggest to not use literal strings here, but have #defines for it. The rationale is quite simple: If you have a typo in the enum, then it will give a compile-time failure. If you have a typo in a literal string that you write dozens of time in your code, it is hard to spot by the human eye and the compiler will not warn/fail. You will only see the error at runtime, which is the worst outcome. -- To view, visit https://gerrit.osmocom.org/13121 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0 Gerrit-Change-Number: 13121 Gerrit-PatchSet: 5 Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-CC: Max <msur...@sysmocom.de> Gerrit-Comment-Date: Sat, 16 Mar 2019 08:01:48 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
