Use the already defined macro to pass the function return address.
Signed-off-by: Davidlohr Bueso <[email protected]>
---
net/core/skbuff.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index af9185d..0d06850 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -116,22 +116,22 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = {
* Keep out of line to prevent kernel bloat.
* __builtin_return_address is not used because it is not always reliable.
*/
-static void skb_panic(struct sk_buff *skb, unsigned int sz, void *addr,
+static void skb_panic(struct sk_buff *skb, unsigned int sz, unsigned long addr,
const char msg[])
{
- pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n",
+ pr_emerg("%s: text:0x%lx len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n",
msg, addr, skb->len, sz, skb->head, skb->data,
(unsigned long)skb->tail, (unsigned long)skb->end,
skb->dev ? skb->dev->name : "<NULL>");
BUG();
}
-static void skb_over_panic(struct sk_buff *skb, unsigned int sz, void *addr)
+static void skb_over_panic(struct sk_buff *skb, unsigned int sz, unsigned long addr)
{
skb_panic(skb, sz, addr, __func__);
}
-static void skb_under_panic(struct sk_buff *skb, unsigned int sz, void *addr)
+static void skb_under_panic(struct sk_buff *skb, unsigned int sz, unsigned long addr)
{
skb_panic(skb, sz, addr, __func__);
}
@@ -646,7 +646,7 @@ void kfree_skb(struct sk_buff *skb)
smp_rmb();
else if (likely(!atomic_dec_and_test(&skb->users)))
return;
- trace_kfree_skb(skb, __builtin_return_address(0));
+ trace_kfree_skb(skb, (void *) _RET_IP_);
__kfree_skb(skb);
}
EXPORT_SYMBOL(kfree_skb);
@@ -1279,7 +1279,7 @@ unsigned char *skb_put(struct sk_buff *skb, unsigned int len)
skb->tail += len;
skb->len += len;
if (unlikely(skb->tail > skb->end))
- skb_over_panic(skb, len, __builtin_return_address(0));
+ skb_over_panic(skb, len, _RET_IP_);
return tmp;
}
EXPORT_SYMBOL(skb_put);
@@ -1298,7 +1298,7 @@ unsigned char *skb_push(struct sk_buff *skb, unsigned int len)
skb->data -= len;
skb->len += len;
if (unlikely(skb->data<skb->head))
- skb_under_panic(skb, len, __builtin_return_address(0));
+ skb_under_panic(skb, len, _RET_IP_);
return skb->data;
}
EXPORT_SYMBOL(skb_push);
--
1.7.11.7
Hello.
On 05/30/2013 01:09 AM, Davidlohr Bueso wrote:
> Use the already defined macro to pass the function return address.
>
> Signed-off-by: Davidlohr Bueso <[email protected]>
> ---
> net/core/skbuff.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index af9185d..0d06850 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -116,22 +116,22 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = {
> * Keep out of line to prevent kernel bloat.
> * __builtin_return_address is not used because it is not always reliable.
> */
> -static void skb_panic(struct sk_buff *skb, unsigned int sz, void *addr,
> +static void skb_panic(struct sk_buff *skb, unsigned int sz, unsigned long addr,
> const char msg[])
> {
> - pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n",
> + pr_emerg("%s: text:0x%lx len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n",
Why not "text:%#lx" as already used in this string? It's equivalent
to "0x%lx".
WBR, Sergei
Sergei Shtylyov <[email protected]> writes:
> Why not "text:%#lx" as already used in this string? It's
> equivalent to "0x%lx".
Well, I don't know the reasoning in this case, but I'd like to note that
those are not strictly equivalent. Personally I find the formatting of 0
annoying enough to avoid %#x for any value which may be 0. It's
especially bad if you try to line up things by adding leading zeros.
I would expect these to produce the same result, but they don't:
printf("0x%02hhx\n", 0);
printf("%#04hhx\n", 0);
Ending up with a 4 digit output when you expect 2 is very confusing. It
doesn't matter that 0 is 0 in any case. Why doesn't the same happen to
1 then? This is just inconsistent behaviour, and I see no valid excuse
for it.
IMHO the single format character saved isn't worth this at all. I'll
continue using 0x%x
Bjørn
On Thu, 2013-05-30 at 13:08 +0200, Bjørn Mork wrote:
> Sergei Shtylyov <[email protected]> writes:
>
> > Why not "text:%#lx" as already used in this string? It's
> > equivalent to "0x%lx".
>
> Well, I don't know the reasoning in this case, but I'd like to note that
> those are not strictly equivalent. Personally I find the formatting of 0
> annoying enough to avoid %#x for any value which may be 0. It's
> especially bad if you try to line up things by adding leading zeros.
Yep, I found that 0x%lx produced the same output as %p. If you'd rather
I use something else instead, I'm happy to update the patch.
Thanks,
Davidlohr
On Thu, 2013-05-30 at 18:11 -0700, Davidlohr Bueso wrote:
> On Thu, 2013-05-30 at 13:08 +0200, Bj?rn Mork wrote:
> > Sergei Shtylyov <[email protected]> writes:
> >
> > > Why not "text:%#lx" as already used in this string? It's
> > > equivalent to "0x%lx".
> >
> > Well, I don't know the reasoning in this case, but I'd like to note that
> > those are not strictly equivalent. Personally I find the formatting of 0
> > annoying enough to avoid %#x for any value which may be 0. It's
> > especially bad if you try to line up things by adding leading zeros.
>
> Yep, I found that 0x%lx produced the same output as %p.
Don't use a standalone gcc compiled program to
determine what the kernel outputs.
lib/vsprintf.c does not output the same. (32 bit)
The kernel output is;
printk("0x%lx\n", 0x100ul) 0x100
printk("%p\n", (void *)0x100ul) 00000100
printk("%#p\n", (void *)0x100ul) 0x00000100
The last one isn't used at all in kernel source. (gcc complains)
It's always "0x%p"
Joe Perches <[email protected]> writes:
> On Thu, 2013-05-30 at 18:11 -0700, Davidlohr Bueso wrote:
>> On Thu, 2013-05-30 at 13:08 +0200, Bjørn Mork wrote:
>> > Sergei Shtylyov <[email protected]> writes:
>> >
>> > > Why not "text:%#lx" as already used in this string? It's
>> > > equivalent to "0x%lx".
>> >
>> > Well, I don't know the reasoning in this case, but I'd like to note that
>> > those are not strictly equivalent. Personally I find the formatting of 0
>> > annoying enough to avoid %#x for any value which may be 0. It's
>> > especially bad if you try to line up things by adding leading zeros.
>>
>> Yep, I found that 0x%lx produced the same output as %p.
>
> Don't use a standalone gcc compiled program to
> determine what the kernel outputs.
That's a very good point. Sorry for mixing this up. You are of course
right. The kernel does everything correctly, so there is no reason not
to use %#x
Bjørn
Hello.
On 31-05-2013 9:20, Joe Perches wrote:
>>>> Why not "text:%#lx" as already used in this string? It's
>>>> equivalent to "0x%lx".
>>> Well, I don't know the reasoning in this case, but I'd like to note that
>>> those are not strictly equivalent. Personally I find the formatting of 0
>>> annoying enough to avoid %#x for any value which may be 0. It's
>>> especially bad if you try to line up things by adding leading zeros.
>> Yep, I found that 0x%lx produced the same output as %p.
> Don't use a standalone gcc compiled program to
> determine what the kernel outputs.
> lib/vsprintf.c does not output the same. (32 bit)
> The kernel output is;
> printk("0x%lx\n", 0x100ul) 0x100
> printk("%p\n", (void *)0x100ul) 00000100
> printk("%#p\n", (void *)0x100ul) 0x00000100
> The last one isn't used at all in kernel source. (gcc complains)
> It's always "0x%p"
I was talking about using "%#lx", not "%#p". I don't see it in your
example.
WBR, Sergei
On Fri, 2013-05-31 at 18:33 +0400, Sergei Shtylyov wrote:
> On 31-05-2013 9:20, Joe Perches wrote:
> > Don't use a standalone gcc compiled program to
> > determine what the kernel outputs.
[]
> > The kernel output is;
>
> > printk("0x%lx\n", 0x100ul) 0x100
> > printk("%p\n", (void *)0x100ul) 00000100
> > printk("%#p\n", (void *)0x100ul) 0x00000100
>
> > The last one isn't used at all in kernel source. (gcc complains)
> > It's always "0x%p"
>
> I was talking about using "%#lx", not "%#p". I don't see it in your
> example.
"0x%lx" and "%#lx" produce the same output in the kernel.
The latter isn't used very often though.
I expect most coders don't know it exists/works.
$ git grep -E "0x%l{0,2}x" | wc -l
12542
$ git grep -E "%#l{0,2}x" | wc -l
1737
(some false positives there of course)
Also, some might expect that "%#08lx", is 10 chars wide,
but it's only 8, so maybe "0x%08lx" is better used.
The "%#08lx" width defect seems pretty common:
$ git grep -E -i "%#0(8|16)l{0,2}x" | wc -l
253
Hello.
On 05/31/2013 08:54 PM, Joe Perches wrote:
>>> Don't use a standalone gcc compiled program to
>>> determine what the kernel outputs.
> []
>>> The kernel output is;
>>> printk("0x%lx\n", 0x100ul) 0x100
>>> printk("%p\n", (void *)0x100ul) 00000100
>>> printk("%#p\n", (void *)0x100ul) 0x00000100
>>> The last one isn't used at all in kernel source. (gcc complains)
>>> It's always "0x%p"
>> I was talking about using "%#lx", not "%#p". I don't see it in your
>> example.
> "0x%lx" and "%#lx" produce the same output in the kernel.
>
> The latter isn't used very often though.
It's already used in the same format string in this case. That's why
I suggested
not to deviate from the existing code.
WBR, Sergei
From: Davidlohr Bueso <[email protected]>
Date: Wed, 29 May 2013 14:09:03 -0700
> Use the already defined macro to pass the function return address.
>
> Signed-off-by: Davidlohr Bueso <[email protected]>
I've decided that we should just leave this alone for now, because
frankly the choice is arbitrary.