> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote: > > ./branches/13/main/http.c, lines 567-568 > > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line567> > > > > Why are the allocation sizes passed in? > > Ashley Sanders wrote: > To account for this handling the error and auth functions. They both > created a header and out buffer, but of different sizes. In a fair and just > world, I would declare a size inside of the ast_http_create_response > function; however, because I do not want to introduce any regressions, I am > leaving the sizes of the each buffer up to the individual calling functions.
Since you are making a common function in this patch you already know the callers and can declare the sizes involved. You can allocate the larger for each buffer from the two callers. ast_str_create() mallocs the initial buffer size. If you put a larger string into it then the buffer is automatically increased. Passing in different buffer sizes doesn't buy much except to potentially avoid a realloc. > On Jan. 26, 2015, 5:44 p.m., rmudgett wrote: > > ./branches/13/main/http.c, line 577 > > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line577> > > > > Just "OOM" here. It was "Auth OOM" and "error OOM" before you made a > > common function. > > Ashley Sanders wrote: > This is just a debug message to show where the OOM occurred (e.g. "Auth" > for the auth function and "error" for the error function); is it not equally > sufficient to say that the error occurred in this function and then use the > debug logs back trace the source? That would have had to happen before this > change, also. I made it say Auth OOM and error OOM earlier to distinguish which ast_debug output the message. Since you are combining the two into a common function, there is only one ast_debug so OOM is sufficient. On the console, the debug messages display the function and line number. In the log file that information is not output which is unfortunate. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4374/#review14289 ----------------------------------------------------------- On Jan. 26, 2015, 2:03 p.m., Ashley Sanders wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4374/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 2:03 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24316 > https://issues.asterisk.org/jira/browse/ASTERISK-24316 > > > Repository: Asterisk > > > Description > ------- > > Currently, all responses from the Asterisk HTTP server contain a [Server] > header that identifies Asterisk and its version (e.g. > "Server:Asterisk/<version>", where <version> is the currently running version > of Asterisk). The preferred behavior is to allow the user to configure an > alternate name to use for the value returned in the [Server] header for HTTP > responses (e.g. "Server:SomeSuperAwesomeServerName"). > > This patch provides a new configuration property, [servername], in http.conf, > that gives users the ability to modify the value that Asterisk uses when > identifying itself. > > By default, the new property is unused, which means that out-of-the-box, the > HTTP server behaves just like it did prior to the patch. Requests to the HTTP > server will generate responses with the old-style [Server] header (e.g. > "Server:Asterisk/<version>", where <version> is the currently running version > of Asterisk). To see the new behavior, you must add the configuration > property, [servername] with some value (an empty value will work, also) to > http.conf. > > Whatever value the HTTP server is holding for the server name can now be seen > through the httpstatus web page > (http://<bindaddr>:<bindport>/<prefix>/httpstatus) (where [bindaddr], > [bindport], and [prefix] are all values configured in http.conf) and the CLI > command: http show status. > > ***Note*** This is just the patch to the Asterisk source. You can find the > review for the Testsuite at: https://reviewboard.asterisk.org/r/4377/ > > > Diffs > ----- > > ./branches/13/main/http.c 431112 > ./branches/13/configs/samples/http.conf.sample 431112 > > Diff: https://reviewboard.asterisk.org/r/4374/diff/ > > > Testing > ------- > > > Thanks, > > Ashley Sanders > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
