Re: [PATCH] return hidden bug
On 10/23/2007 01:00 AM, Ray Lee wrote: On 10/22/07, Rene Herman <[EMAIL PROTECTED]> wrote: Hugely trust inspiring isn't it -- the amount of eyes and comments you'll get even on trivial patches like this? This development model is working! Go easy with the snarkiness, hmm? It's the trivial ones that seem to be the most dangerous. The larger ones actually get *tested* sometimes. I was also commenting and don't generally on anything much more involved so any snarkiness included myself which should make things better. Now if only we'd sometimes get some for non trivial patches as well... That's certainly true regardless, but for myself, I'd rather throw some reviews out for the small ones since I have adequate time and knowledge for them. The larger ones require domain specific knowledge I lack, and time I don't have. Exactly the problem. Comment was mostly triggered due to me looking at a problem with a proprietary CD-ROM driver again tonight that I posted a few months ago where the only comment has been from the fellow author. There the problem was the block layer blowing up and given that it seems unlikely that this wouldn't be a problem inside the newbie-driver itself, that the block part of it was actually really small and people said they'd look at it, the subsequent thundering silence still annoys me. Ofcourse, now it seems the kernel itself has moved on enough that the driver doesn't work at _all_ anymore and I at the moment lack the time to spend the required hours googling around trying to find out what the heck changed out from under me so that I might get it to at least do what it already did do. Hey, I don't actually know and maybe I'm just wrong but I have the feeling that over the last 1 or 2 years most new developers seem to be either people that are payed to be so, perhaps in the form of graduation, or janitors. The kernel is much, much more complex than even only a few years ago and at the same time the number of knowledgeable developers who'll do something other than their own thing and otherwise just wait around for something perfect to merge seems to be approaching zero. That is -- I do not feel that the current developer base is expending overly many efforts to appear welcoming. Please feel free to do the open-source thing and argue that's actually an advantage (there we have that snarkiness again...) or otherwise ignore me. I'll just sit here and be grumpy anyway. Might be better after a good night's sleep... Rene. - 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] return hidden bug
On 10/22/07, Rene Herman <[EMAIL PROTECTED]> wrote: > Hugely trust inspiring isn't it -- the amount of eyes and comments you'll > get even on trivial patches like this? This development model is working! Go easy with the snarkiness, hmm? It's the trivial ones that seem to be the most dangerous. The larger ones actually get *tested* sometimes. > Now if only we'd sometimes get some for non trivial patches as well... That's certainly true regardless, but for myself, I'd rather throw some reviews out for the small ones since I have adequate time and knowledge for them. The larger ones require domain specific knowledge I lack, and time I don't have. ~r. - 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] return hidden bug
On 10/22/2007 08:52 PM, Roel Kluin wrote: Ray Lee wrote: Arguing intentions is very dangerous. I've written code like that where the intention is to make it simple to turn a printk into a full bug and back and forth during development. At the end of the day, the fact remains that you're changing behavior. Let me turn this around. Do you have an alpha and have you tried out your patch? If not, then I'd suggest turning it into a WARN_ON(1) instead, as in this specific case you're risking turning what was a working system into one that doesn't. No, I haven't and, I will change it, but it's included with my other changes. see the reply that I'll write shortly for. [PATCH retry] return hidden bug and unlock bugs. Hugely trust inspiring isn't it -- the amount of eyes and comments you'll get even on trivial patches like this? This development model is working! Now if only we'd sometimes get some for non trivial patches as well... Rene. - 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] return hidden bug
Ray Lee wrote: >>> I'm sorry, perhaps I poured myself a cup of stupid this morning, but >>> isn't the above patch effectively introducing a BUG where none could >>> be reached before? In other words, for the patch to have zero >>> behavioral change, wouldn't it have to remove the BUG() altogether? >> True, but obviously not intended. I think the intention was to expose this >> bug. > > Arguing intentions is very dangerous. I've written code like that > where the intention is to make it simple to turn a printk into a full > bug and back and forth during development. At the end of the day, the > fact remains that you're changing behavior. > > Let me turn this around. Do you have an alpha and have you tried out > your patch? If not, then I'd suggest turning it into a WARN_ON(1) > instead, as in this specific case you're risking turning what was a > working system into one that doesn't. No, I haven't and, I will change it, but it's included with my other changes. see the reply that I'll write shortly for. [PATCH retry] return hidden bug and unlock bugs. Roel - 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] return hidden bug
On Sun, Oct 21, 2007 at 09:42:09PM -0400, Rik van Riel wrote: > On Mon, 22 Oct 2007 03:05:05 +0200 > Roel Kluin <[EMAIL PROTECTED]> wrote: > > > return hidden bug > > > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > > > > diff --git a/arch/alpha/kernel/pci_iommu.c > > b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 > > --- a/arch/alpha/kernel/pci_iommu.c > > +++ b/arch/alpha/kernel/pci_iommu.c > > @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t > > dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: > > dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, > >arena->size); > > - return; > > BUG(); > > + return; > > } > > > > npages = calc_npages((dma_addr & ~PAGE_MASK) + size); > > BUG() will terminate the process that runs into it, so you can > just remove the return alltogether. If BUG() is hit, the return > will never be reached. Looking at the printk, I don't think this particular error ought to forcefully kill things. -- Mathematics is the supreme nostalgia of our time. - 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] return hidden bug
On 10/22/07, Roel Kluin <[EMAIL PROTECTED]> wrote: > Ray Lee wrote: > > > I'm sorry, perhaps I poured myself a cup of stupid this morning, but > > isn't the above patch effectively introducing a BUG where none could > > be reached before? In other words, for the patch to have zero > > behavioral change, wouldn't it have to remove the BUG() altogether? > > True, but obviously not intended. I think the intention was to expose this > bug. Arguing intentions is very dangerous. I've written code like that where the intention is to make it simple to turn a printk into a full bug and back and forth during development. At the end of the day, the fact remains that you're changing behavior. Let me turn this around. Do you have an alpha and have you tried out your patch? If not, then I'd suggest turning it into a WARN_ON(1) instead, as in this specific case you're risking turning what was a working system into one that doesn't. - 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] return hidden bug
Ray Lee wrote: > I'm sorry, perhaps I poured myself a cup of stupid this morning, but > isn't the above patch effectively introducing a BUG where none could > be reached before? In other words, for the patch to have zero > behavioral change, wouldn't it have to remove the BUG() altogether? True, but obviously not intended. I think the intention was to expose this bug. - 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] return hidden bug
On 10/21/07, Rik van Riel <[EMAIL PROTECTED]> wrote: > On Mon, 22 Oct 2007 03:05:05 +0200 > Roel Kluin <[EMAIL PROTECTED]> wrote: > > > return hidden bug > > > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > > > > diff --git a/arch/alpha/kernel/pci_iommu.c > > b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 > > --- a/arch/alpha/kernel/pci_iommu.c > > +++ b/arch/alpha/kernel/pci_iommu.c > > @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t > > dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: > > dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, > > arena->size); > > - return; > > BUG(); > > + return; > > } > > > > npages = calc_npages((dma_addr & ~PAGE_MASK) + size); > > BUG() will terminate the process that runs into it, so you can > just remove the return alltogether. If BUG() is hit, the return > will never be reached. I'm sorry, perhaps I poured myself a cup of stupid this morning, but isn't the above patch effectively introducing a BUG where none could be reached before? In other words, for the patch to have zero behavioral change, wouldn't it have to remove the BUG() altogether? Ray - 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] return hidden bug
Hi Rik, On 10/22/07, Rik van Riel <[EMAIL PROTECTED]> wrote: > On Mon, 22 Oct 2007 12:30:00 +0300 > I guess people who disable CONFIG_BUG really choose to shoot themselves > in the foot when something bad happens. The kernel is full of error > paths where the current thread really should not be continuing. Sure, if you disable BUG_ON, it may or may not work. But making it *worse* on purpose seems pointless. - 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] return hidden bug
On Mon, 22 Oct 2007 12:30:00 +0300 "Pekka Enberg" <[EMAIL PROTECTED]> wrote: > On 10/22/07, Rik van Riel <[EMAIL PROTECTED]> wrote: > > BUG() will terminate the process that runs into it, so you can > > just remove the return alltogether. If BUG() is hit, the return > > will never be reached. > > This isn't true when CONFIG_BUG is disabled (in embedded builds, for > example). *blink* I guess people who disable CONFIG_BUG really choose to shoot themselves in the foot when something bad happens. The kernel is full of error paths where the current thread really should not be continuing. Oh well... -- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan - 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] return hidden bug
Hi, On 10/22/07, Rik van Riel <[EMAIL PROTECTED]> wrote: > BUG() will terminate the process that runs into it, so you can > just remove the return alltogether. If BUG() is hit, the return > will never be reached. This isn't true when CONFIG_BUG is disabled (in embedded builds, for example). Pekka - 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] return hidden bug
Hi, On 10/22/07, Rik van Riel [EMAIL PROTECTED] wrote: BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. This isn't true when CONFIG_BUG is disabled (in embedded builds, for example). Pekka - 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] return hidden bug
On Mon, 22 Oct 2007 12:30:00 +0300 Pekka Enberg [EMAIL PROTECTED] wrote: On 10/22/07, Rik van Riel [EMAIL PROTECTED] wrote: BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. This isn't true when CONFIG_BUG is disabled (in embedded builds, for example). *blink* I guess people who disable CONFIG_BUG really choose to shoot themselves in the foot when something bad happens. The kernel is full of error paths where the current thread really should not be continuing. Oh well... -- Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. - Brian W. Kernighan - 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] return hidden bug
Hi Rik, On 10/22/07, Rik van Riel [EMAIL PROTECTED] wrote: On Mon, 22 Oct 2007 12:30:00 +0300 I guess people who disable CONFIG_BUG really choose to shoot themselves in the foot when something bad happens. The kernel is full of error paths where the current thread really should not be continuing. Sure, if you disable BUG_ON, it may or may not work. But making it *worse* on purpose seems pointless. - 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] return hidden bug
On 10/21/07, Rik van Riel [EMAIL PROTECTED] wrote: On Mon, 22 Oct 2007 03:05:05 +0200 Roel Kluin [EMAIL PROTECTED] wrote: return hidden bug Signed-off-by: Roel Kluin [EMAIL PROTECTED] diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); - return; BUG(); + return; } npages = calc_npages((dma_addr ~PAGE_MASK) + size); BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. I'm sorry, perhaps I poured myself a cup of stupid this morning, but isn't the above patch effectively introducing a BUG where none could be reached before? In other words, for the patch to have zero behavioral change, wouldn't it have to remove the BUG() altogether? Ray - 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] return hidden bug
Ray Lee wrote: I'm sorry, perhaps I poured myself a cup of stupid this morning, but isn't the above patch effectively introducing a BUG where none could be reached before? In other words, for the patch to have zero behavioral change, wouldn't it have to remove the BUG() altogether? True, but obviously not intended. I think the intention was to expose this bug. - 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] return hidden bug
On 10/22/07, Roel Kluin [EMAIL PROTECTED] wrote: Ray Lee wrote: I'm sorry, perhaps I poured myself a cup of stupid this morning, but isn't the above patch effectively introducing a BUG where none could be reached before? In other words, for the patch to have zero behavioral change, wouldn't it have to remove the BUG() altogether? True, but obviously not intended. I think the intention was to expose this bug. Arguing intentions is very dangerous. I've written code like that where the intention is to make it simple to turn a printk into a full bug and back and forth during development. At the end of the day, the fact remains that you're changing behavior. Let me turn this around. Do you have an alpha and have you tried out your patch? If not, then I'd suggest turning it into a WARN_ON(1) instead, as in this specific case you're risking turning what was a working system into one that doesn't. - 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] return hidden bug
On Sun, Oct 21, 2007 at 09:42:09PM -0400, Rik van Riel wrote: On Mon, 22 Oct 2007 03:05:05 +0200 Roel Kluin [EMAIL PROTECTED] wrote: return hidden bug Signed-off-by: Roel Kluin [EMAIL PROTECTED] diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); - return; BUG(); + return; } npages = calc_npages((dma_addr ~PAGE_MASK) + size); BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. Looking at the printk, I don't think this particular error ought to forcefully kill things. -- Mathematics is the supreme nostalgia of our time. - 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] return hidden bug
Ray Lee wrote: I'm sorry, perhaps I poured myself a cup of stupid this morning, but isn't the above patch effectively introducing a BUG where none could be reached before? In other words, for the patch to have zero behavioral change, wouldn't it have to remove the BUG() altogether? True, but obviously not intended. I think the intention was to expose this bug. Arguing intentions is very dangerous. I've written code like that where the intention is to make it simple to turn a printk into a full bug and back and forth during development. At the end of the day, the fact remains that you're changing behavior. Let me turn this around. Do you have an alpha and have you tried out your patch? If not, then I'd suggest turning it into a WARN_ON(1) instead, as in this specific case you're risking turning what was a working system into one that doesn't. No, I haven't and, I will change it, but it's included with my other changes. see the reply that I'll write shortly for. [PATCH retry] return hidden bug and unlock bugs. Roel - 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] return hidden bug
On 10/22/2007 08:52 PM, Roel Kluin wrote: Ray Lee wrote: Arguing intentions is very dangerous. I've written code like that where the intention is to make it simple to turn a printk into a full bug and back and forth during development. At the end of the day, the fact remains that you're changing behavior. Let me turn this around. Do you have an alpha and have you tried out your patch? If not, then I'd suggest turning it into a WARN_ON(1) instead, as in this specific case you're risking turning what was a working system into one that doesn't. No, I haven't and, I will change it, but it's included with my other changes. see the reply that I'll write shortly for. [PATCH retry] return hidden bug and unlock bugs. Hugely trust inspiring isn't it -- the amount of eyes and comments you'll get even on trivial patches like this? This development model is working! Now if only we'd sometimes get some for non trivial patches as well... Rene. - 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] return hidden bug
On 10/22/07, Rene Herman [EMAIL PROTECTED] wrote: Hugely trust inspiring isn't it -- the amount of eyes and comments you'll get even on trivial patches like this? This development model is working! Go easy with the snarkiness, hmm? It's the trivial ones that seem to be the most dangerous. The larger ones actually get *tested* sometimes. Now if only we'd sometimes get some for non trivial patches as well... That's certainly true regardless, but for myself, I'd rather throw some reviews out for the small ones since I have adequate time and knowledge for them. The larger ones require domain specific knowledge I lack, and time I don't have. ~r. - 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] return hidden bug
On 10/23/2007 01:00 AM, Ray Lee wrote: On 10/22/07, Rene Herman [EMAIL PROTECTED] wrote: Hugely trust inspiring isn't it -- the amount of eyes and comments you'll get even on trivial patches like this? This development model is working! Go easy with the snarkiness, hmm? It's the trivial ones that seem to be the most dangerous. The larger ones actually get *tested* sometimes. I was also commenting and don't generally on anything much more involved so any snarkiness included myself which should make things better. Now if only we'd sometimes get some for non trivial patches as well... That's certainly true regardless, but for myself, I'd rather throw some reviews out for the small ones since I have adequate time and knowledge for them. The larger ones require domain specific knowledge I lack, and time I don't have. Exactly the problem. Comment was mostly triggered due to me looking at a problem with a proprietary CD-ROM driver again tonight that I posted a few months ago where the only comment has been from the fellow author. There the problem was the block layer blowing up and given that it seems unlikely that this wouldn't be a problem inside the newbie-driver itself, that the block part of it was actually really small and people said they'd look at it, the subsequent thundering silence still annoys me. Ofcourse, now it seems the kernel itself has moved on enough that the driver doesn't work at _all_ anymore and I at the moment lack the time to spend the required hours googling around trying to find out what the heck changed out from under me so that I might get it to at least do what it already did do. Hey, I don't actually know and maybe I'm just wrong but I have the feeling that over the last 1 or 2 years most new developers seem to be either people that are payed to be so, perhaps in the form of graduation, or janitors. The kernel is much, much more complex than even only a few years ago and at the same time the number of knowledgeable developers who'll do something other than their own thing and otherwise just wait around for something perfect to merge seems to be approaching zero. That is -- I do not feel that the current developer base is expending overly many efforts to appear welcoming. Please feel free to do the open-source thing and argue that's actually an advantage (there we have that snarkiness again...) or otherwise ignore me. I'll just sit here and be grumpy anyway. Might be better after a good night's sleep... Rene. - 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] return hidden bug
On Mon, Oct 22, 2007 at 01:42:09AM +, Rik van Riel wrote: > > BUG() will terminate the process that runs into it, so you can > just remove the return alltogether. If BUG() is hit, the return > will never be reached. This is true in general. However, if someone builds the kernel with CONFIG_BUG disabled then the return statement will make a difference. Of course, if you're brave enough to have CONFIG_BUG off then you can deal with the fall-out :) Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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] return hidden bug
On Mon, 22 Oct 2007 03:53:30 +0200 Roel Kluin <[EMAIL PROTECTED]> wrote: > hidden bug returns > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> Acked-by: Rik van Riel <[EMAIL PROTECTED]> -- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan - 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] return hidden bug
Rik van Riel wrote: > On Mon, 22 Oct 2007 03:05:05 +0200 > Roel Kluin <[EMAIL PROTECTED]> wrote: > >> return hidden bug >> >> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> >> >> diff --git a/arch/alpha/kernel/pci_iommu.c >> b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 >> --- a/arch/alpha/kernel/pci_iommu.c >> +++ b/arch/alpha/kernel/pci_iommu.c >> @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t >> dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: >> dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, >> arena->size); >> -return; >> BUG(); >> +return; >> } >> >> npages = calc_npages((dma_addr & ~PAGE_MASK) + size); > > BUG() will terminate the process that runs into it, so you can > just remove the return alltogether. If BUG() is hit, the return > will never be reached. > --- hidden bug returns Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..ca55c33 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,7 +365,6 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, arena->size); - return; BUG(); } - 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] return hidden bug
On Mon, 22 Oct 2007 03:05:05 +0200 Roel Kluin <[EMAIL PROTECTED]> wrote: > return hidden bug > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > > diff --git a/arch/alpha/kernel/pci_iommu.c > b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 > --- a/arch/alpha/kernel/pci_iommu.c > +++ b/arch/alpha/kernel/pci_iommu.c > @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t > dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: > dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, > arena->size); > - return; > BUG(); > + return; > } > > npages = calc_npages((dma_addr & ~PAGE_MASK) + size); BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. -- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan - 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/
[PATCH] return hidden bug
return hidden bug Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx " " base %lx size %x\n", dma_addr, arena->dma_base, arena->size); - return; BUG(); + return; } npages = calc_npages((dma_addr & ~PAGE_MASK) + size); - 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/
[PATCH] return hidden bug
return hidden bug Signed-off-by: Roel Kluin [EMAIL PROTECTED] diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); - return; BUG(); + return; } npages = calc_npages((dma_addr ~PAGE_MASK) + size); - 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] return hidden bug
On Mon, 22 Oct 2007 03:05:05 +0200 Roel Kluin [EMAIL PROTECTED] wrote: return hidden bug Signed-off-by: Roel Kluin [EMAIL PROTECTED] diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); - return; BUG(); + return; } npages = calc_npages((dma_addr ~PAGE_MASK) + size); BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. -- Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. - Brian W. Kernighan - 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] return hidden bug
Rik van Riel wrote: On Mon, 22 Oct 2007 03:05:05 +0200 Roel Kluin [EMAIL PROTECTED] wrote: return hidden bug Signed-off-by: Roel Kluin [EMAIL PROTECTED] diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..6a69425 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); -return; BUG(); +return; } npages = calc_npages((dma_addr ~PAGE_MASK) + size); BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. --- hidden bug returns Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c index e1c4707..ca55c33 100644 --- a/arch/alpha/kernel/pci_iommu.c +++ b/arch/alpha/kernel/pci_iommu.c @@ -365,7 +365,6 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, printk(KERN_ERR Bogus pci_unmap_single: dma_addr %lx base %lx size %x\n, dma_addr, arena-dma_base, arena-size); - return; BUG(); } - 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] return hidden bug
On Mon, 22 Oct 2007 03:53:30 +0200 Roel Kluin [EMAIL PROTECTED] wrote: hidden bug returns Signed-off-by: Roel Kluin [EMAIL PROTECTED] Acked-by: Rik van Riel [EMAIL PROTECTED] -- Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. - Brian W. Kernighan - 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] return hidden bug
On Mon, Oct 22, 2007 at 01:42:09AM +, Rik van Riel wrote: BUG() will terminate the process that runs into it, so you can just remove the return alltogether. If BUG() is hit, the return will never be reached. This is true in general. However, if someone builds the kernel with CONFIG_BUG disabled then the return statement will make a difference. Of course, if you're brave enough to have CONFIG_BUG off then you can deal with the fall-out :) Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED] Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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/