Re: [PATCH net 9/9] net: qed: fix "maybe uninitialized" warning
From: Jakub Kicinski
Date: Mon, 22 Jun 2020 14:44:37 -0700
> On Mon, 22 Jun 2020 14:14:13 +0300 Alexander Lobakin wrote:
> > Variable 'abs_ppfid' in qed_dev.c:qed_llh_add_mac_filter() always gets
> > printed, but is initialized only under 'ref_cnt == 1' condition. This
> > results in:
> >
> > In file included from ./include/linux/kernel.h:15:0,
> > from ./include/asm-generic/bug.h:19,
> > from ./arch/x86/include/asm/bug.h:86,
> > from ./include/linux/bug.h:5,
> > from ./include/linux/io.h:11,
> > from drivers/net/ethernet/qlogic/qed/qed_dev.c:35:
> > drivers/net/ethernet/qlogic/qed/qed_dev.c: In function
> > 'qed_llh_add_mac_filter':
> > ./include/linux/printk.h:358:2: warning: 'abs_ppfid' may be used
> > uninitialized
> > in this function [-Wmaybe-uninitialized]
> > printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > ^~
> > drivers/net/ethernet/qlogic/qed/qed_dev.c:983:17: note: 'abs_ppfid' was
> > declared
> > here
> > u8 filter_idx, abs_ppfid;
> > ^
> >
> > ...under W=1+.
> >
> > Fix this by initializing it with zero.
> >
> > Fixes: 79284adeb99e ("qed: Add llh ppfid interface and 100g support for
> > offload protocols")
> > Signed-off-by: Alexander Lobakin
> > Signed-off-by: Igor Russkikh
> > Signed-off-by: Michal Kalderon
>
> Please don't wrap Fixes tags:
Aww, second time in a row I fail on this. Sorry, will send v2
soon.
> Fixes tag: Fixes: 79284adeb99e ("qed: Add llh ppfid interface and 100g
> support for
> Has these problem(s):
> - Subject has leading but no trailing parentheses
> - Subject has leading but no trailing quotes
Al
Re: [PATCH net 9/9] net: qed: fix "maybe uninitialized" warning
On Mon, 22 Jun 2020 14:14:13 +0300 Alexander Lobakin wrote:
> Variable 'abs_ppfid' in qed_dev.c:qed_llh_add_mac_filter() always gets
> printed, but is initialized only under 'ref_cnt == 1' condition. This
> results in:
>
> In file included from ./include/linux/kernel.h:15:0,
> from ./include/asm-generic/bug.h:19,
> from ./arch/x86/include/asm/bug.h:86,
> from ./include/linux/bug.h:5,
> from ./include/linux/io.h:11,
> from drivers/net/ethernet/qlogic/qed/qed_dev.c:35:
> drivers/net/ethernet/qlogic/qed/qed_dev.c: In function
> 'qed_llh_add_mac_filter':
> ./include/linux/printk.h:358:2: warning: 'abs_ppfid' may be used uninitialized
> in this function [-Wmaybe-uninitialized]
> printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> ^~
> drivers/net/ethernet/qlogic/qed/qed_dev.c:983:17: note: 'abs_ppfid' was
> declared
> here
> u8 filter_idx, abs_ppfid;
> ^
>
> ...under W=1+.
>
> Fix this by initializing it with zero.
>
> Fixes: 79284adeb99e ("qed: Add llh ppfid interface and 100g support for
> offload protocols")
> Signed-off-by: Alexander Lobakin
> Signed-off-by: Igor Russkikh
> Signed-off-by: Michal Kalderon
Please don't wrap Fixes tags:
Fixes tag: Fixes: 79284adeb99e ("qed: Add llh ppfid interface and 100g support
for
Has these problem(s):
- Subject has leading but no trailing parentheses
- Subject has leading but no trailing quotes
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 2017年01月04日 00:40, John Fastabend wrote:
On 17-01-02 10:16 PM, Jason Wang wrote:
On 2017年01月03日 06:43, John Fastabend wrote:
On 16-12-23 06:37 AM, Jason Wang wrote:
Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
small receive buffer untouched. This will confuse the user who want to
set XDP but use small buffers. Other than forbid XDP in small buffer
mode, let's make it work. XDP then can only work at skb->data since
virtio-net create skbs during refill, this is sub optimal which could
be optimized in the future.
Cc: John Fastabend
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 112
---
1 file changed, 87 insertions(+), 25 deletions(-)
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
We probably need a better name for this function.
Acked-by: Jason Wang
How about is_xdp_raw_buffer_queue()?
I'll submit a proper patch today.
Sounds good, thanks.
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 2017年01月04日 00:40, John Fastabend wrote:
On 17-01-02 10:16 PM, Jason Wang wrote:
On 2017年01月03日 06:43, John Fastabend wrote:
On 16-12-23 06:37 AM, Jason Wang wrote:
Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
small receive buffer untouched. This will confuse the user who want to
set XDP but use small buffers. Other than forbid XDP in small buffer
mode, let's make it work. XDP then can only work at skb->data since
virtio-net create skbs during refill, this is sub optimal which could
be optimized in the future.
Cc: John Fastabend
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 112
---
1 file changed, 87 insertions(+), 25 deletions(-)
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
We probably need a better name for this function.
Acked-by: Jason Wang
How about is_xdp_raw_buffer_queue()?
I'll submit a proper patch today.
Sounds good, thanks.
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 17-01-02 10:16 PM, Jason Wang wrote:
>
>
> On 2017年01月03日 06:43, John Fastabend wrote:
>> On 16-12-23 06:37 AM, Jason Wang wrote:
>>> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
>>> small receive buffer untouched. This will confuse the user who want to
>>> set XDP but use small buffers. Other than forbid XDP in small buffer
>>> mode, let's make it work. XDP then can only work at skb->data since
>>> virtio-net create skbs during refill, this is sub optimal which could
>>> be optimized in the future.
>>>
>>> Cc: John Fastabend
>>> Signed-off-by: Jason Wang
>>> ---
>>> drivers/net/virtio_net.c | 112
>>> ---
>>> 1 file changed, 87 insertions(+), 25 deletions(-)
>>>
>> Hi Jason,
>>
>> I was doing some more testing on this what do you think about doing this
>> so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
>> instead of put_page in small receive mode. Seems more correct to me.
>>
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 783e842..27ff76c 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct
>> virtnet_info
>> *vi)
>>
>> static bool is_xdp_queue(struct virtnet_info *vi, int q)
>> {
>> + /* For small receive mode always use kfree_skb variants */
>> + if (!vi->mergeable_rx_bufs)
>> + return false;
>> +
>> if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
>> return false;
>> else if (q < vi->curr_queue_pairs)
>>
>>
>> patch is untested just spotted doing code review.
>>
>> Thanks,
>> John
>
> We probably need a better name for this function.
>
> Acked-by: Jason Wang
>
How about is_xdp_raw_buffer_queue()?
I'll submit a proper patch today.
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 17-01-02 10:16 PM, Jason Wang wrote:
>
>
> On 2017年01月03日 06:43, John Fastabend wrote:
>> On 16-12-23 06:37 AM, Jason Wang wrote:
>>> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
>>> small receive buffer untouched. This will confuse the user who want to
>>> set XDP but use small buffers. Other than forbid XDP in small buffer
>>> mode, let's make it work. XDP then can only work at skb->data since
>>> virtio-net create skbs during refill, this is sub optimal which could
>>> be optimized in the future.
>>>
>>> Cc: John Fastabend
>>> Signed-off-by: Jason Wang
>>> ---
>>> drivers/net/virtio_net.c | 112
>>> ---
>>> 1 file changed, 87 insertions(+), 25 deletions(-)
>>>
>> Hi Jason,
>>
>> I was doing some more testing on this what do you think about doing this
>> so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
>> instead of put_page in small receive mode. Seems more correct to me.
>>
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 783e842..27ff76c 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct
>> virtnet_info
>> *vi)
>>
>> static bool is_xdp_queue(struct virtnet_info *vi, int q)
>> {
>> + /* For small receive mode always use kfree_skb variants */
>> + if (!vi->mergeable_rx_bufs)
>> + return false;
>> +
>> if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
>> return false;
>> else if (q < vi->curr_queue_pairs)
>>
>>
>> patch is untested just spotted doing code review.
>>
>> Thanks,
>> John
>
> We probably need a better name for this function.
>
> Acked-by: Jason Wang
>
How about is_xdp_raw_buffer_queue()?
I'll submit a proper patch today.
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 2017年01月03日 06:43, John Fastabend wrote:
On 16-12-23 06:37 AM, Jason Wang wrote:
Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
small receive buffer untouched. This will confuse the user who want to
set XDP but use small buffers. Other than forbid XDP in small buffer
mode, let's make it work. XDP then can only work at skb->data since
virtio-net create skbs during refill, this is sub optimal which could
be optimized in the future.
Cc: John Fastabend
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 112 ---
1 file changed, 87 insertions(+), 25 deletions(-)
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
We probably need a better name for this function.
Acked-by: Jason Wang
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 2017年01月03日 06:43, John Fastabend wrote:
On 16-12-23 06:37 AM, Jason Wang wrote:
Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
small receive buffer untouched. This will confuse the user who want to
set XDP but use small buffers. Other than forbid XDP in small buffer
mode, let's make it work. XDP then can only work at skb->data since
virtio-net create skbs during refill, this is sub optimal which could
be optimized in the future.
Cc: John Fastabend
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 112 ---
1 file changed, 87 insertions(+), 25 deletions(-)
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
We probably need a better name for this function.
Acked-by: Jason Wang
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 16-12-23 06:37 AM, Jason Wang wrote:
> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
> small receive buffer untouched. This will confuse the user who want to
> set XDP but use small buffers. Other than forbid XDP in small buffer
> mode, let's make it work. XDP then can only work at skb->data since
> virtio-net create skbs during refill, this is sub optimal which could
> be optimized in the future.
>
> Cc: John Fastabend
> Signed-off-by: Jason Wang
> ---
> drivers/net/virtio_net.c | 112
> ---
> 1 file changed, 87 insertions(+), 25 deletions(-)
>
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 16-12-23 06:37 AM, Jason Wang wrote:
> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
> small receive buffer untouched. This will confuse the user who want to
> set XDP but use small buffers. Other than forbid XDP in small buffer
> mode, let's make it work. XDP then can only work at skb->data since
> virtio-net create skbs during refill, this is sub optimal which could
> be optimized in the future.
>
> Cc: John Fastabend
> Signed-off-by: Jason Wang
> ---
> drivers/net/virtio_net.c | 112
> ---
> 1 file changed, 87 insertions(+), 25 deletions(-)
>
Hi Jason,
I was doing some more testing on this what do you think about doing this
so that free_unused_bufs() handles the buffer free with dev_kfree_skb()
instead of put_page in small receive mode. Seems more correct to me.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 783e842..27ff76c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1898,6 +1898,10 @@ static void free_receive_page_frags(struct virtnet_info
*vi)
static bool is_xdp_queue(struct virtnet_info *vi, int q)
{
+ /* For small receive mode always use kfree_skb variants */
+ if (!vi->mergeable_rx_bufs)
+ return false;
+
if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
return false;
else if (q < vi->curr_queue_pairs)
patch is untested just spotted doing code review.
Thanks,
John
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 16-12-23 06:37 AM, Jason Wang wrote:
> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
> small receive buffer untouched. This will confuse the user who want to
> set XDP but use small buffers. Other than forbid XDP in small buffer
> mode, let's make it work. XDP then can only work at skb->data since
> virtio-net create skbs during refill, this is sub optimal which could
> be optimized in the future.
>
> Cc: John Fastabend
> Signed-off-by: Jason Wang
> ---
Looks good to me thanks!
Acked-by: John Fastabend
Re: [PATCH net 9/9] virtio-net: XDP support for small buffers
On 16-12-23 06:37 AM, Jason Wang wrote:
> Commit f600b6905015 ("virtio_net: Add XDP support") leaves the case of
> small receive buffer untouched. This will confuse the user who want to
> set XDP but use small buffers. Other than forbid XDP in small buffer
> mode, let's make it work. XDP then can only work at skb->data since
> virtio-net create skbs during refill, this is sub optimal which could
> be optimized in the future.
>
> Cc: John Fastabend
> Signed-off-by: Jason Wang
> ---
Looks good to me thanks!
Acked-by: John Fastabend
Re: [PATCH] net #9
"Henning P. Schmiedehausen wrote:"
> Andrzej Krzysztofowicz <[EMAIL PROTECTED]> writes:
>
> >-static char name[4][IFNAMSIZ] = { "", "", "", "" };
>
> >+static char name[4][IFNAMSIZ];
>
> Ugh. Sure about that one? the variables have been pointers to zero,
> now they're zero...
I do not agree. As I understand C "name" is a table, i.e. a pointer to a
prealocated area of size of 4*IFNAMSIZ*sizeof(char) bytes. There is no
pointers to the strings stored seprately.
[ The strings are copied into the apropriate locations in the array during
initialization ]
After applying the mentioned patch just the whole area of the array will be
zeroed. Not only the first characters of name[i] i=0,1,2,3 ...
Andrzej
--
===
Andrzej M. Krzysztofowicz [EMAIL PROTECTED]
phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math., Technical University of Gdansk
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
"Henning P. Schmiedehausen" wrote:
>
> Andrzej Krzysztofowicz <[EMAIL PROTECTED]> writes:
>
> >-static char name[4][IFNAMSIZ] = { "", "", "", "" };
>
> >+static char name[4][IFNAMSIZ];
>
> Ugh. Sure about that one? the variables have been pointers to zero,
> now they're zero...
No, the variables were and always have been arrays, not pointers.
The previous incarnation, {"","","",""}, was actually doubly lame,
because not only was it an unnecessary zeroing of the var, but using ""
causes an extra string to be generated in the output asm.
--
Jeff Garzik | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
Henning P. Schmiedehausen wrote:
Andrzej Krzysztofowicz [EMAIL PROTECTED] writes:
-static char name[4][IFNAMSIZ] = { , , , };
+static char name[4][IFNAMSIZ];
Ugh. Sure about that one? the variables have been pointers to zero,
now they're zero...
No, the variables were and always have been arrays, not pointers.
The previous incarnation, {,,,}, was actually doubly lame,
because not only was it an unnecessary zeroing of the var, but using
causes an extra string to be generated in the output asm.
--
Jeff Garzik | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
Henning P. Schmiedehausen wrote:
Andrzej Krzysztofowicz [EMAIL PROTECTED] writes:
-static char name[4][IFNAMSIZ] = { , , , };
+static char name[4][IFNAMSIZ];
Ugh. Sure about that one? the variables have been pointers to zero,
now they're zero...
I do not agree. As I understand C name is a table, i.e. a pointer to a
prealocated area of size of 4*IFNAMSIZ*sizeof(char) bytes. There is no
pointers to the strings stored seprately.
[ The strings are copied into the apropriate locations in the array during
initialization ]
After applying the mentioned patch just the whole area of the array will be
zeroed. Not only the first characters of name[i] i=0,1,2,3 ...
Andrzej
--
===
Andrzej M. Krzysztofowicz [EMAIL PROTECTED]
phone (48)(58) 347 14 61
Faculty of Applied Phys. Math., Technical University of Gdansk
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 10:13:02PM -0400, Jeff Garzik wrote: > *shrug* Well, if you want to go against the kernel standard that's fine > with me. I won't put Andrzej's changes to your drivers upstream. You > are going to continually see patches to clean that up, though, because > it makes the end user's kernel smaller. Please consider noting this > special case in a comment in each of your drivers "do not clean up > static initializers" or similar. > > It's really a pain in the ass to remember special cases like this, so > please reconsider. Being not-like-the-others is detrimental to the long > term maintainability of the overall kernel. > > Regards, > > Jeff I agree with you on the special case. I don't like it either. Anyway, most patch to my drivers are applied wether I like it or not, so I guess that I should be happy that I was notified and I should sut up my big mouth because it won't make a difference. If I reject the patch now, I will be applied behind my back. Been there, done that. In other words : yes, please apply the patch. Jean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 09:47:19PM -0400, Jeff Garzik wrote: > > This is ANSI C standard stuff. If a static object with a scalar type is > not explicitly initialized, it is initialized to zero by default. > > Sure we can get gcc to recognize that case, but why use gcc to work > around code that avoids an ANSI feature? Apart from this stupid flame that I'm making, I've got my Intel/Symbol card to work properly with the Orinoco driver. This mean that we are not far away to have the 4 main flavor of 802.11b working in 2.4.X (i.e. Lucent/Symbol/PrismII/Aironet). See : http://www.hpl.hp.com/personal/Jean_Tourrilhes/Linux/Orinoco.html#patches Just to make sure we end on a positive note ;-) Now, if I could get the card of Alan to work... Have fun, don't take it seriously... Jean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
Jean Tourrilhes wrote: > > On Tue, May 29, 2001 at 09:13:34PM -0400, Jeff Garzik wrote: > > > > This is standard kernel cleanup that makes the resulting image smaller. > > These patches have been going into all areas of the kernel for quite > > some time. > > This doesn't make it right. > > Ok, while we are on the topic : could somebody explain me why > we can't get gcc to do that for us ? What is preventing adding a gcc > command line flag to do exactly that ? It's not like rocket science > (simple test) and would avoid to have tediously to go through all > source code, past, present and *future* to make those changes. > Bah, maybe it's too straightforward... This is ANSI C standard stuff. If a static object with a scalar type is not explicitly initialized, it is initialized to zero by default. Sure we can get gcc to recognize that case, but why use gcc to work around code that avoids an ANSI feature? -- Jeff Garzik | Disbelief, that's why you fail. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 09:13:34PM -0400, Jeff Garzik wrote: > > This is standard kernel cleanup that makes the resulting image smaller. > These patches have been going into all areas of the kernel for quite > some time. This doesn't make it right. Ok, while we are on the topic : could somebody explain me why we can't get gcc to do that for us ? What is preventing adding a gcc command line flag to do exactly that ? It's not like rocket science (simple test) and would avoid to have tediously to go through all source code, past, present and *future* to make those changes. Bah, maybe it's too straightforward... Jean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Wed, May 30, 2001 at 02:48:24AM +0200, Andrzej Krzysztofowicz wrote: > > The following patch removes some zero initializers from statics > > Andrzej If I were you, I would fix gcc rather than making my code unreadable. I write source code in C rather than coding ASM in hex because of the semantic attached to what I write, which make the code readable by me and by other, and make my code portable to other environments (for example user space). Initialisating a variable to zero as opposed to leaving it undefined has plenty of semantic attached to it. It's the job of the compiler to make sure that all this kind of stupid optimisation are done and the code produced is as efficient as possible and adapted to the exact characteristics of the operating envirtonment. Especially that it's probably 10 lines to add the proper option to gcc command line. Therefore, Alan, please do not apply those kind of patches to my drivers. Thanks... Jean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 09:13:34PM -0400, Jeff Garzik wrote: This is standard kernel cleanup that makes the resulting image smaller. These patches have been going into all areas of the kernel for quite some time. This doesn't make it right. Ok, while we are on the topic : could somebody explain me why we can't get gcc to do that for us ? What is preventing adding a gcc command line flag to do exactly that ? It's not like rocket science (simple test) and would avoid to have tediously to go through all source code, past, present and *future* to make those changes. Bah, maybe it's too straightforward... Jean - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 10:13:02PM -0400, Jeff Garzik wrote: *shrug* Well, if you want to go against the kernel standard that's fine with me. I won't put Andrzej's changes to your drivers upstream. You are going to continually see patches to clean that up, though, because it makes the end user's kernel smaller. Please consider noting this special case in a comment in each of your drivers do not clean up static initializers or similar. It's really a pain in the ass to remember special cases like this, so please reconsider. Being not-like-the-others is detrimental to the long term maintainability of the overall kernel. Regards, Jeff I agree with you on the special case. I don't like it either. Anyway, most patch to my drivers are applied wether I like it or not, so I guess that I should be happy that I was notified and I should sut up my big mouth because it won't make a difference. If I reject the patch now, I will be applied behind my back. Been there, done that. In other words : yes, please apply the patch. Jean - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
Jean Tourrilhes wrote: On Tue, May 29, 2001 at 09:13:34PM -0400, Jeff Garzik wrote: This is standard kernel cleanup that makes the resulting image smaller. These patches have been going into all areas of the kernel for quite some time. This doesn't make it right. Ok, while we are on the topic : could somebody explain me why we can't get gcc to do that for us ? What is preventing adding a gcc command line flag to do exactly that ? It's not like rocket science (simple test) and would avoid to have tediously to go through all source code, past, present and *future* to make those changes. Bah, maybe it's too straightforward... This is ANSI C standard stuff. If a static object with a scalar type is not explicitly initialized, it is initialized to zero by default. Sure we can get gcc to recognize that case, but why use gcc to work around code that avoids an ANSI feature? -- Jeff Garzik | Disbelief, that's why you fail. Building 1024| MandrakeSoft | - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Tue, May 29, 2001 at 09:47:19PM -0400, Jeff Garzik wrote: This is ANSI C standard stuff. If a static object with a scalar type is not explicitly initialized, it is initialized to zero by default. Sure we can get gcc to recognize that case, but why use gcc to work around code that avoids an ANSI feature? Apart from this stupid flame that I'm making, I've got my Intel/Symbol card to work properly with the Orinoco driver. This mean that we are not far away to have the 4 main flavor of 802.11b working in 2.4.X (i.e. Lucent/Symbol/PrismII/Aironet). See : http://www.hpl.hp.com/personal/Jean_Tourrilhes/Linux/Orinoco.html#patches Just to make sure we end on a positive note ;-) Now, if I could get the card of Alan to work... Have fun, don't take it seriously... Jean - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net #9
On Wed, May 30, 2001 at 02:48:24AM +0200, Andrzej Krzysztofowicz wrote: The following patch removes some zero initializers from statics Andrzej If I were you, I would fix gcc rather than making my code unreadable. I write source code in C rather than coding ASM in hex because of the semantic attached to what I write, which make the code readable by me and by other, and make my code portable to other environments (for example user space). Initialisating a variable to zero as opposed to leaving it undefined has plenty of semantic attached to it. It's the job of the compiler to make sure that all this kind of stupid optimisation are done and the code produced is as efficient as possible and adapted to the exact characteristics of the operating envirtonment. Especially that it's probably 10 lines to add the proper option to gcc command line. Therefore, Alan, please do not apply those kind of patches to my drivers. Thanks... Jean - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
