On 07/17/2014 02:27 PM, Andrey Utkin wrote: > static int pkt_type_offset(void) returned -1 in case of failure, and > actual (positive) offset value in case of success. In the only instance > of its usage, the result was saved to local "unsigned int off" variable, > which is used in a lot of places in the same (large) function, so > changing its type could cause many warnings. > There was no signed int variable which could be just used for this case. > There are two possibilities to resolve that: to declare temporary signed > int variable to get the return value from pkt_type_offset(), or to > separate return status from returned offset value. The latter approach > was chosen, however, I am not sure which would be practically optimal. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=80371 > Reported-by: David Binderman <dcb314@xxxxxxxxxxx> > Signed-off-by: Andrey Utkin <andrey.krieger.utkin@xxxxxxxxx> > --- > arch/mips/net/bpf_jit.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c > index b87390a..47f65d5 100644 > --- a/arch/mips/net/bpf_jit.c > +++ b/arch/mips/net/bpf_jit.c > @@ -770,7 +770,7 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset) > #else > #define PKT_TYPE_MAX 7 > #endif > -static int pkt_type_offset(void) > +static int pkt_type_offset(unsigned int *off_arg) > { > struct sk_buff skb_probe = { > .pkt_type = ~0, > @@ -779,8 +779,10 @@ static int pkt_type_offset(void) > unsigned int off; > > for (off = 0; off < sizeof(struct sk_buff); off++) { > - if (ct[off] == PKT_TYPE_MAX) > - return off; > + if (ct[off] == PKT_TYPE_MAX) { > + *off_arg = off; > + return 0; > + } > } > pr_err_once("Please fix pkt_type_offset(), as pkt_type couldn't be found\n"); > return -1; > @@ -1332,9 +1334,7 @@ jmp_cmp: > case BPF_ANC | SKF_AD_PKTTYPE: > ctx->flags |= SEEN_SKB; > > - off = pkt_type_offset(); > - > - if (off < 0) > + if (pkt_type_offset(&off)) > return -1; > emit_load_byte(r_tmp, r_skb, off, ctx); > /* Keep only the last 3 bits */ > Hi, Thanks for the patch. I would personally prefer to use a new signed int variable, but I am fine either way. -- markos
