{"thread":{"id":"44911","subject":"[PATCH] attr: mark a file-local symbol as static","startedAt":"2017-01-18T22:34:05Z","lastAt":"2017-01-19T01:23:00Z","messageCount":3,"participants":["Ramsay Jones","Brandon Williams"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"309686","messageId":"89290015-7c5f-1a5d-e683-59077ae55bf5@ramsayjones.plus.com","threadId":"44911","inReplyTo":null,"subject":"[PATCH] attr: mark a file-local symbol as static","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-01-18T22:27:57Z","receivedAt":"2017-01-18T22:34:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Brandon,\n\nIf you need to re-roll your 'bw/attr' branch, could you please\nsquash this into the relevant patch (commit 8908457159,\n\"attr: use hashmap for attribute dictionary\", 12-01-2017).\n\nAlso, I note that, although they are declared as part of the\npublic attr api, attr_check_clear() and attr_check_reset() are\nalso not called outside of attr.c. Are these functions part of\nthe public api?\n\nAlso, a minor point, but attr_check_reset() is called (line 1050)\nbefore it's definition (line 1114). This is not a problem, given\nthe declaration in attr.h, but I prefer definitions to come before\nuse, where possible.\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n attr.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/attr.c b/attr.c\nindex f5cc68b67..e68c4688f 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -83,7 +83,7 @@ static int attr_hash_entry_cmp(const struct attr_hash_entry *a,\n }\n \n /* Initialize an 'attr_hashmap' object */\n-void attr_hashmap_init(struct attr_hashmap *map)\n+static void attr_hashmap_init(struct attr_hashmap *map)\n {\n \thashmap_init(&map->map, (hashmap_cmp_fn) attr_hash_entry_cmp, 0);\n }\n-- \n2.11.0\n"},{"id":"309690","messageId":"20170118230541.GE10641@google.com","threadId":"44911","inReplyTo":"89290015-7c5f-1a5d-e683-59077ae55bf5@ramsayjones.plus.com","subject":"Re: [PATCH] attr: mark a file-local symbol as static","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-01-18T23:05:41Z","receivedAt":"2017-01-18T23:06:49Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 01/18, Ramsay Jones wrote:\n> \n> Signed-off-by: Ramsay Jones \n> ---\n> \n> Hi Brandon,\n> \n> If you need to re-roll your 'bw/attr' branch, could you please\n> squash this into the relevant patch (commit 8908457159,\n> \"attr: use hashmap for attribute dictionary\", 12-01-2017).\n> \n> Also, I note that, although they are declared as part of the\n> public attr api, attr_check_clear() and attr_check_reset() are\n> also not called outside of attr.c. Are these functions part of\n> the public api?\n> \n> Also, a minor point, but attr_check_reset() is called (line 1050)\n> before it's definition (line 1114). This is not a problem, given\n> the declaration in attr.h, but I prefer definitions to come before\n> use, where possible.\n> \n> Thanks!\n> \n> ATB,\n> Ramsay Jones\n\nYes of course, I believe Stefan also pointed that out earlier today so I\nhave it fixed locally.\n\nFor attr_check_clear() and attr_check_reset() the intent is that they\nare the accepted way to either clear or reset the attr_check structure.\nCurrently most users of the attribute system don't have a need to clear\nor reset the structure but there could be future callers who need that\nfunctionality. If you feel like they shouldn't be part of the api right\nnow then I'm open to changing that for this series.\n\nThanks!\n\n-- \nBrandon Williams\n"},{"id":"309706","messageId":"993c0278-03ae-a28a-df29-beed9864a4ae@ramsayjones.plus.com","threadId":"44911","inReplyTo":"20170118230541.GE10641@google.com","subject":"Re: [PATCH] attr: mark a file-local symbol as static","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-01-19T01:22:45Z","receivedAt":"2017-01-19T01:23:00Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 18/01/17 23:05, Brandon Williams wrote:\n> On 01/18, Ramsay Jones wrote:\n>>\n>> Signed-off-by: Ramsay Jones \n>> ---\n>>\n>> Hi Brandon,\n>>\n>> If you need to re-roll your 'bw/attr' branch, could you please\n>> squash this into the relevant patch (commit 8908457159,\n>> \"attr: use hashmap for attribute dictionary\", 12-01-2017).\n>>\n>> Also, I note that, although they are declared as part of the\n>> public attr api, attr_check_clear() and attr_check_reset() are\n>> also not called outside of attr.c. Are these functions part of\n>> the public api?\n>>\n>> Also, a minor point, but attr_check_reset() is called (line 1050)\n>> before it's definition (line 1114). This is not a problem, given\n>> the declaration in attr.h, but I prefer definitions to come before\n>> use, where possible.\n>>\n>> Thanks!\n>>\n>> ATB,\n>> Ramsay Jones\n> \n> Yes of course, I believe Stefan also pointed that out earlier today so I\n> have it fixed locally.\n\nYep, I noticed Stefan's email just a few minutes after I hit\nsend! ;-)\n\n> For attr_check_clear() and attr_check_reset() the intent is that they\n> are the accepted way to either clear or reset the attr_check structure.\n> Currently most users of the attribute system don't have a need to clear\n> or reset the structure but there could be future callers who need that\n> functionality. If you feel like they shouldn't be part of the api right\n> now then I'm open to changing that for this series.\n\nNo, I just wanted to check that they were intended to be part of\nthe public api and that you anticipate additional callers in the\nfuture.\n\n[I would still prefer definitions before use, but many people would\nnot agree with me, so ...]\n\nATB,\nRamsay Jones\n\n\n"}]}