<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I do agree though that if this approach is the only one chosen then a comment as mentioned should be added.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Agreed.  Happy to add that, but not if:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Since there are two patch already implemented is there any reason to not commit both?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">No, technically both does apply cleanly and can be merged.</p><p style="white-space: pre-wrap; word-wrap: break-word;">However, if both gets merged, then as per yourself, the comment isn't required, or at least the comment should be different in order to state that's it's not *preferable* to access argv directly using static indexes, however, without looking at the actual code, I suspect argv[] is required in order to actually place the char* pointers correctly.  One could of course use other hacks and eliminate argv[] completely from the struct, but those would be less clean in my opinion.  The other possible advantage of applying the other patch is that AST_STANDARD_APP_ARGS (and friends) could be updated to the cleaner/more concise/more clear ARRAY_LEN macro directly on argv.  This would also be "safer" in the sense that if for some reason some additional fields ends up getting added *AFTER* the argv+arglist "union", that will still work.</p><p style="white-space: pre-wrap; word-wrap: break-word;">There is another "pattern" I've noticed, for example in func_odbc whereby 100 args gets created arbitrarily, since that can either use it's "own" array, or argv, one could conceivably create a parse_and_dynamically_allocate_args function, and then a free_dynamic_args which purely relies on argv[0].  I suspect this may be in potential conflict with the other patch to a degree, the advantage of such an approach would be genuinly arbitrary argument count, as well as not waste stack space since these structs are generally allocated on the stack, but, it would come at the cost of a memory allocation + free, and additional extra pointer indirections.  I don't think this is a high priority - anybody dealing with 100+ arguments in the dialplan has got bigger problems in my opinion.  Additionally, this would REQUIRE that the argv "union" be the last in the struct (which of course can be enforced with a compile time assert (typedef void* __argv_must_be_last_element_of_args_struct[(boolean expr returning true if ct assert is true)-1];) but IMHO the effort isn't worth the gain currently.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15496">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15496">change 15496</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15496"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I75152cece8a00b7523d542e5ac22796f9595692b </div>
<div style="display:none"> Gerrit-Change-Number: 15496 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-CC: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 04 Mar 2021 09:22:53 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>