> On Jan. 26, 2015, 4:55 p.m., Scott Griepentrog wrote: > > /branches/13/apps/confbridge/conf_config_parser.c, line 2304 > > <https://reviewboard.asterisk.org/r/4372/diff/1/?file=71080#file71080line2304> > > > > Use ast_copy_string? > > rmudgett wrote: > I second this should be ast_copy_string() for safety. > > Matt Jordan wrote: > There's no need to use ast_copy_string if the buffers are guaranteed to > be the same length, which is the case for both structs. > > To quote further from the Coding Guidelines: > > {quote} > Don't use ast_copy_string (or any length-limited copy function) for > copying fixed (known at compile time) strings into buffers, if the buffer is > something that has been allocated in the function doing the copying. In that > case, you know at the time you are writing the code whether the buffer is > large enough for the fixed string or not, and if it's not, your code won't > work anyway! Use strcpy() for this operation, or directly set the first two > characters of the buffer if you are just trying to store a one character > string in the buffer. If you are trying to 'empty' the buffer, just store a > single NULL character ('\0') in the first byte of the buffer; nothing else is > needed, and any other method is wasteful. > {quote} > > While that's referring to a slightly different situation, we know that > both the source and destination are the same length. There's no need to use > ast_copy_string.
True, but the sizes are defined elsewhere and not allocated right here in this function. At the very least the /* Safe */ comment should be added like elsewhere. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4372/#review14290 ----------------------------------------------------------- On Jan. 26, 2015, 7:33 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4372/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 7:33 p.m.) > > > Review request for Asterisk Developers and Jonathan Rose. > > > Bugs: ASTERISK-24723 > https://issues.asterisk.org/jira/browse/ASTERISK-24723 > > > Repository: Asterisk > > > Description > ------- > > When issuing a 'confbridge list XXXX' CLI command, the resulting output no > longer displays the menu associated with a ConfBridge participant: > > Channel Flags User Profile Bridge Profile Menu > CallerID > ============================== ====== ================ ================ > ================ ================ > PJSIP/1601-00000004 default_user default_bridge > 1601 > > The issue was caused by ASTERISK-22760. When that patch was done, it removed > the copying of the menu name associated with the user from the actual user > profile. > > This patch fixes the issue by copying the menu name over to the user profile > when the menu hooks are applied to the user. Since that function now does a > little bit more than just apply the hooks, the name of the function has been > changed to cover the copying of the menu name over as well. > > In addition, there is a disparity between the menu name length as it is > stored on the conf_menu structure and the confbridge_user structure; this > patch makes the lengths match so that a strcpy can be used. > > > Diffs > ----- > > /branches/13/apps/confbridge/include/confbridge.h 431019 > /branches/13/apps/confbridge/conf_config_parser.c 431019 > > Diff: https://reviewboard.asterisk.org/r/4372/diff/ > > > Testing > ------- > > Ran the CLI command. The output now shows the menu associated with the user. > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- 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
