[Asterisk-code-review] main/cli.c: Refactor function to print seconds formatted (asterisk[master])

Walter Doekes asteriskteam at digium.com
Mon Feb 22 06:33:03 CST 2016


Walter Doekes has posted comments on this change.

Change subject: main/cli.c: Refactor function to print seconds formatted
......................................................................


Patch Set 3:

(1 comment)

Thanks for the changes!

Instead of re-adding the missing initializers to the needed elements, you've added them to all elements. I won't reject it over that.

But explaining the buffer size calculation in this review instead of in the source is not where it belongs, IMO. (See below.)

Further, you've added a space before a trailing-semicolon in the update coded. (There should be an automatic linter that complains about newly introduced style issues.)

https://gerrit.asterisk.org/#/c/2272/3/res/res_config_pgsql.c
File res/res_config_pgsql.c:

Line 1579: 	char buf[376];
> From  length connection_buffer + credential + "Connected to  for " + 1
Then please document it as such.

    char buf[376]; /* 256+100+"Connected to "+" for "+NUL */

If you're using narrow buffers like that, someone who whishes to change a text element below will be able to recalculate it without having to inspect all the code.


-- 
To view, visit https://gerrit.asterisk.org/2272
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb8634102cd11d3f8623398b279cb731bcde36c
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Rodrigo Ramirez Norambuena <a at rodrigoramirez.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Rodrigo Ramirez Norambuena <a at rodrigoramirez.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list