> On Oct. 1, 2014, 10:47 a.m., rmudgett wrote:
> > /trunk/apps/app_queue.c, lines 8364-8366
> > <https://reviewboard.asterisk.org/r/4035/diff/2-4/?file=67798#file67798line8364>
> >
> > guidelines: variable declarations go at the beginning of a block.
> >
> > struct call_queue tmpq;
> > ...
> > memset(&tmpq, 0, sizeof(tmpq));
> > tmpq.name = args.queuename;
> >
>
> Kristian Høgh wrote:
> Make gives me: assignment of read-only member ‘name’
> I'm unable to fix it.
>
> wdoekes wrote:
> I don't think not-using-(designated)-struct-initializers is part of the
> guidelines.
>
> memset+assignment are less efficient and less readable.
>
> And since the stringfield stuff makes it a const `const ast_string_field
> name;`
> the above won't work without an ugly cast.
wdokes:
Declaring variables within a code block is against guidelines. Not to mention
that the compiler generates a warning about it that prevents the file from
compiling in developemnt mode. Using struct initializers is not against
guidelines. They are used all over Asterisk.
Using memset+assignment is not that inefficient nor that less readable.
This problem can be fixed readily enough by declaring tmpq in a local block
just before using it like this:
...
{
struct call_queue tmpq = {
.name = args.queuename,
};
q = ao2_t_find(queues, &tmpq, ....);
}
if (q) {
...
}
- rmudgett
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4035/#review13429
-----------------------------------------------------------
On Oct. 2, 2014, 4:17 a.m., Kristian Høgh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4035/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 4:17 a.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-24365
> https://issues.asterisk.org/jira/browse/ASTERISK-24365
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> The function returns the caller at specified position in a queue.
>
> Noop(${QUEUE_GET_CHANNEL(queuename), 2})
> "SIP/peer-00000002", if queue exist and have at least 2 callers
>
> Noop(${QUEUE_GET_CHANNEL(queuename), 1})
> "SIP/peer-00000000", if queue exist and have at least 1 caller
>
>
> Before diff r3:
> The function returns the first caller in a specific queue.
> Noop(${QUEUE_GET_FIRSTCHANNEL(queuename)})
> "SIP/peer-00000002", if queue exist and is non-empty
>
>
> Diffs
> -----
>
> /trunk/apps/app_queue.c 424149
>
> Diff: https://reviewboard.asterisk.org/r/4035/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kristian Høgh
>
>
--
_____________________________________________________________________
-- 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
