<p> Attention is currently required from: Sean Bright, Stanislav Abramenkov, George Joseph, Kevin Harwell, Benjamin Keith Ford. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16665">View Change</a></p><p>17 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/utils.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/ed2762c3_564383da">Patch Set #11, Line 347:</a> <code style="font-family:monospace,monospace">int ast_base64_encode_file(FILE *inputfile, FILE *outputfile, char *endl);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Make endl const</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/39bc6858_60b846e6">Patch Set #11, Line 355:</a> <code style="font-family:monospace,monospace"> * \return zero on success, -1 on error.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Change this to '\retval 0' vs return zero</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I did change return to retval but the merge conflict screwed up some of my changes I think... trying to figure this out :)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/48a66764_99c130a7">Patch Set #11, Line 357:</a> <code style="font-family:monospace,monospace">int ast_base64_encode_file_path(const char *filename, FILE *outputfile, char *endl);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">make endl const</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/utils.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/7778e998_ce659704">Patch Set #10, Line 344:</a> <code style="font-family:monospace,monospace"> * \return zero on success, -1 on error.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Make this \retval instead of \return.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/4d4b681b_ea7493dc">Patch Set #10, Line 346:</a> <code style="font-family:monospace,monospace">ast_file_base_encode</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Personally I think this should be renamed to ast_base64_encode_file. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/39aa5052_076fc730">Patch Set #10, Line 346:</a> <code style="font-family:monospace,monospace">so</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">rename this to output_file, or something more obvious that it's the file being written to.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/cc305398_3856057c">Patch Set #10, Line 346:</a> <code style="font-family:monospace,monospace">char *endl</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Make this a const, and document the parameter</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/37d5455c_08784afd">Patch Set #10, Line 346:</a> <code style="font-family:monospace,monospace">int ast_file_base_encode(char *filename, FILE *so, char *endl);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I agree with Sean though, in that since this is now a public API call this should probably accept tw […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/ed76f704_17ee4643">Patch Set #10, Line 346:</a> <code style="font-family:monospace,monospace">char *filename</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Make this a const</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/678e013d_cf75c55c">Patch Set #10, Line 347:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br>/*!<br> * \brief same as mkstemp, but return a FILE<br> * \param template The template for the unique file name to generate. Modified in place to return the file name.<br> * \param mode The mode for file permissions<br> * \param mask The mask for file permissions<br> *<br> * \return FILE handle to the temporary file on success or NULL if creation failed<br> */<br>FILE *ast_file_mkftemp(char *template, mode_t mode, mode_t mask);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'm thinking this whole function should be moved to file. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/utils.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/a65d382c_1c330323">Patch Set #10, Line 592:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if (bio->ateof)<br> return 0;<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I know this was from before, but since moving mind as well bring the code up to current guidelines. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/c0a414e1_bfd8ed7f">Patch Set #10, Line 614:</a> <code style="font-family:monospace,monospace"> if (bio->iocp>=bio->iolen) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">add a space before and after >=</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/7c9bce4e_dd01d90e">Patch Set #10, Line 615:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if (!inbuf(bio, fi))<br> return EOF;<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">braces here too</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/854bbdb6_58ae714c">Patch Set #10, Line 644:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">/*!<br> * \brief Performs a base 64 encode algorithm on the contents of a File<br> * \param filename The path to the file to be encoded. Must be readable, file is opened in read mode.<br> * \param so A FILE handle to the output file to receive the base 64 encoded contents of the input file, identified by filename.<br> *<br> * \return zero on success, -1 on error.<br> */<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Remove this comment since it's documented publicly.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/d42d9c30_f16aa02f">Patch Set #10, Line 693:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if (n < 2)<br> ogroup[2] = '=';<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Add braces to "if"</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/8cc97227_677c15ce">Patch Set #10, Line 697:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> for (i = 0; i < 4; i++)<br> ochar(&bio, ogroup[i], so, endl);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Add braces to "for"</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16665/comment/7e39eaa8_7764a1ed">Patch Set #10, Line 711:</a> <code style="font-family:monospace,monospace">/* same as mkstemp, but return a FILE * */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Remove comment</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16665">change 16665</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/+/16665"/><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: I1de0ed3483623e9599711129edc817c45ad237ee </div>
<div style="display:none"> Gerrit-Change-Number: 16665 </div>
<div style="display:none"> Gerrit-PatchSet: 14 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </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-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-CC: Stanislav Abramenkov <stas.abramenkov@gmail.com> </div>
<div style="display:none"> Gerrit-Attention: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Stanislav Abramenkov <stas.abramenkov@gmail.com> </div>
<div style="display:none"> Gerrit-Attention: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:01:32 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>