<p>Corey Farrell <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/7471">View Change</a></p><p>Patch set 1:</p><p style="white-space: pre-wrap; word-wrap: break-word;">I wrote this before you updated the patch.  I haven't reviewed the bulk changes yet, couple thoughts/nits on the new function itself.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Nice patch BTW.  I always found fcntl to be awkward.</p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/7471/1/main/utils.c">File main/utils.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/7471/1/main/utils.c@2802">Patch Set #1, Line 2802:</a> <code style="font-family:monospace,monospace">int _ast_fd_set_flags(int fd, int flags, int set, const char *file, int lineno)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">1) Should this function be in main/astfd.c (not a statement a question).<br>2) Why not include the caller's function name?<br>3) Can you use __ast_fd_set_flags to go with normal debug variant naming?</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/7471/1/main/utils.c@2808">Patch Set #1, Line 2808:</a> <code style="font-family:monospace,monospace">               ast_log(LOG_ERROR, "%s:%d: Failed to get flags for file descriptor: %s\n",</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Any reason you didn't use:<br>ast_log(__LOG_ERROR, file, func, line, "Failed to get ...", strerror(errno))?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also (nit/ignore if you want): I think "Failed to get fcntl flags .." would improve the message - that would tell me immediately what the problem was.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/7471">change 7471</a>. To unsubscribe, 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/7471"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I8b81901e1b1bd537ca632567cdb408931c6eded7 </div>
<div style="display:none"> Gerrit-Change-Number: 7471 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 07 Dec 2017 17:05:50 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>