On 21 October 2014 11:57, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Tue, Oct 21, 2014 at 11:56:49AM +0200, Pablo Neira Ayuso wrote:
>> On Tue, Oct 21, 2014 at 10:53:19AM +0200, Arturo Borrero Gonzalez wrote:
>> > On 21 October 2014 09:59, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>> > >
>> > > BTW, it would be good to see a similar refactor in nft_verdict2str().
>> >
>> > I don't see a clean way to do it, given some verdicts are negative
>> > numbers (enum nft_verdicts in nf_tables.h).
>> > We may end accessing a negative index, out of bounds of the array.
>>
>> I see, you mean:
>>
>> enum nft_verdicts {
>> NFT_CONTINUE = -1,
>> NFT_BREAK = -2,
>> NFT_JUMP = -3,
>> NFT_GOTO = -4,
>> NFT_RETURN = -5,
>> };
>>
>> You can add some function to shift the values:
>>
>> #define nft_verdict_index(base) (base + 5)
>
> BTW, instead of 5, add:
>
> #define NFT_VERDICT_BASE NFT_RETURN
>
> and use it.
>
>>
>> ... nft_verdict_array[] = {
>> [nft_verdict_index(NFT_RETURN)] = "return",
>> ...
>> };
Ok, following your idea, I end with something like this:
/*
* NF_DROP = 0
* NF_ACCEPT = 1
* NFT_JUMP = -3
* NFT_GOTO = -4
* NFT_RETURN = -5
*/
#define NFT_VERDICT_BASE -NFT_RETURN
#define nft_verdict_index(base) (base + NFT_VERDICT_BASE)
#define NFT_VERDICT_ARRAY_LEN nft_verdict_index(NF_ACCEPT)
static const char *const nft_verdict_str[NFT_VERDICT_ARRAY_LEN + 1] = {
[nft_verdict_index(NFT_RETURN)] = "return", /* 0 */
[nft_verdict_index(NFT_GOTO)] = "goto", /* 1 */
[nft_verdict_index(NFT_JUMP)] = "jump", /* 2 */
[nft_verdict_index(NF_DROP)] = "drop", /* 5 */
[nft_verdict_index(NF_ACCEPT)] = "accept", /* 6 */
};
const char *nft_verdict2str(int verdict)
{
if (nft_verdict_str[nft_verdict_index(verdict)] == NULL)
return "unknown";
return nft_verdict_str[nft_verdict_index(verdict)];
}
int nft_str2verdict(const char *verdict, int *verdict_num)
{
int i;
for (i = 0; i < NFT_VERDICT_ARRAY_LEN; i++) {
if (nft_verdict_str[i] == NULL)
continue;
if (strcmp(nft_verdict_str[i], verdict) == 0) {
*verdict_num = nft_verdict_index(i);
return 0;
}
}
return -1;
}
I think the current code (the switch based one) is better, don't you?
That array seems very error prone.
--
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
