Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
From: "Srivatsa S. Bhat" Date: Wed, 12 Sep 2012 13:54:05 +0530 > On 09/12/2012 01:19 PM, David Miller wrote: >> From: "Srivatsa S. Bhat" >> Date: Wed, 12 Sep 2012 11:37:47 +0530 >> >>> + memcpy(new_priomap->priomap, old_priomap->priomap, >>> + old_priomap->priomap_len * >>> + sizeof(old_priomap->priomap[0])); >> >> This argument indentation is ridiculous. Try: >> >> memcpy(new_priomap->priomap, old_priomap->priomap, >> old_priomap->priomap_len * >> sizeof(old_priomap->priomap[0])); >> >> Using TABs exclusively for argumentat indentation is not the goal. >> >> Rather, lining the arguments up properly so that they sit at the first >> column after the first line's openning parenthesis is what you should >> be trying to achieve. > > OK, will fix it, thanks! > >> >> And ignoring whatever stylistic convention we may or may not have, I >> find it impossibly hard to believe that the code quoted above looks >> good even to you. >> > > On second thoughts, I think the memcpy in this case will actually be worse > since it will copy the contents in chunks of smaller size than the for-loop. I meant just coding style wise, not semantically. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
On 09/12/2012 01:54 PM, Srivatsa S. Bhat wrote: > On 09/12/2012 01:19 PM, David Miller wrote: >> From: "Srivatsa S. Bhat" >> Date: Wed, 12 Sep 2012 11:37:47 +0530 >> >>> + memcpy(new_priomap->priomap, old_priomap->priomap, >>> + old_priomap->priomap_len * >>> + sizeof(old_priomap->priomap[0])); >> >> This argument indentation is ridiculous. Try: >> >> memcpy(new_priomap->priomap, old_priomap->priomap, >> old_priomap->priomap_len * >> sizeof(old_priomap->priomap[0])); >> >> Using TABs exclusively for argumentat indentation is not the goal. >> >> Rather, lining the arguments up properly so that they sit at the first >> column after the first line's openning parenthesis is what you should >> be trying to achieve. > > OK, will fix it, thanks! > >> >> And ignoring whatever stylistic convention we may or may not have, I >> find it impossibly hard to believe that the code quoted above looks >> good even to you. >> > > On second thoughts, I think the memcpy in this case will actually be worse > since it will copy the contents in chunks of smaller size than the for-loop. Oops, I missed the __HAVE_ARCH_MEMCPY and was looking at the wrong memcpy implementation.. And in any case, I went totally off-track by your last comment. I hadn't realized that you were still referring to the way the code looks, rather than questioning the switch to memcpy. Sorry about that! I'll fix the odd-looking indentation and repost the patch. Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
On 09/12/2012 01:19 PM, David Miller wrote: > From: "Srivatsa S. Bhat" > Date: Wed, 12 Sep 2012 11:37:47 +0530 > >> +memcpy(new_priomap->priomap, old_priomap->priomap, >> +old_priomap->priomap_len * >> +sizeof(old_priomap->priomap[0])); > > This argument indentation is ridiculous. Try: > > memcpy(new_priomap->priomap, old_priomap->priomap, > old_priomap->priomap_len * > sizeof(old_priomap->priomap[0])); > > Using TABs exclusively for argumentat indentation is not the goal. > > Rather, lining the arguments up properly so that they sit at the first > column after the first line's openning parenthesis is what you should > be trying to achieve. OK, will fix it, thanks! > > And ignoring whatever stylistic convention we may or may not have, I > find it impossibly hard to believe that the code quoted above looks > good even to you. > On second thoughts, I think the memcpy in this case will actually be worse since it will copy the contents in chunks of smaller size than the for-loop. Or, did you mean to say that this code is plain wrong for some reason? Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
From: "Srivatsa S. Bhat" Date: Wed, 12 Sep 2012 11:37:47 +0530 > + memcpy(new_priomap->priomap, old_priomap->priomap, > + old_priomap->priomap_len * > + sizeof(old_priomap->priomap[0])); This argument indentation is ridiculous. Try: memcpy(new_priomap->priomap, old_priomap->priomap, old_priomap->priomap_len * sizeof(old_priomap->priomap[0])); Using TABs exclusively for argumentat indentation is not the goal. Rather, lining the arguments up properly so that they sit at the first column after the first line's openning parenthesis is what you should be trying to achieve. And ignoring whatever stylistic convention we may or may not have, I find it impossibly hard to believe that the code quoted above looks good even to you. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com Date: Wed, 12 Sep 2012 11:37:47 +0530 + memcpy(new_priomap-priomap, old_priomap-priomap, + old_priomap-priomap_len * + sizeof(old_priomap-priomap[0])); This argument indentation is ridiculous. Try: memcpy(new_priomap-priomap, old_priomap-priomap, old_priomap-priomap_len * sizeof(old_priomap-priomap[0])); Using TABs exclusively for argumentat indentation is not the goal. Rather, lining the arguments up properly so that they sit at the first column after the first line's openning parenthesis is what you should be trying to achieve. And ignoring whatever stylistic convention we may or may not have, I find it impossibly hard to believe that the code quoted above looks good even to you. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
On 09/12/2012 01:19 PM, David Miller wrote: From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com Date: Wed, 12 Sep 2012 11:37:47 +0530 +memcpy(new_priomap-priomap, old_priomap-priomap, +old_priomap-priomap_len * +sizeof(old_priomap-priomap[0])); This argument indentation is ridiculous. Try: memcpy(new_priomap-priomap, old_priomap-priomap, old_priomap-priomap_len * sizeof(old_priomap-priomap[0])); Using TABs exclusively for argumentat indentation is not the goal. Rather, lining the arguments up properly so that they sit at the first column after the first line's openning parenthesis is what you should be trying to achieve. OK, will fix it, thanks! And ignoring whatever stylistic convention we may or may not have, I find it impossibly hard to believe that the code quoted above looks good even to you. On second thoughts, I think the memcpy in this case will actually be worse since it will copy the contents in chunks of smaller size than the for-loop. Or, did you mean to say that this code is plain wrong for some reason? Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
On 09/12/2012 01:54 PM, Srivatsa S. Bhat wrote: On 09/12/2012 01:19 PM, David Miller wrote: From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com Date: Wed, 12 Sep 2012 11:37:47 +0530 + memcpy(new_priomap-priomap, old_priomap-priomap, + old_priomap-priomap_len * + sizeof(old_priomap-priomap[0])); This argument indentation is ridiculous. Try: memcpy(new_priomap-priomap, old_priomap-priomap, old_priomap-priomap_len * sizeof(old_priomap-priomap[0])); Using TABs exclusively for argumentat indentation is not the goal. Rather, lining the arguments up properly so that they sit at the first column after the first line's openning parenthesis is what you should be trying to achieve. OK, will fix it, thanks! And ignoring whatever stylistic convention we may or may not have, I find it impossibly hard to believe that the code quoted above looks good even to you. On second thoughts, I think the memcpy in this case will actually be worse since it will copy the contents in chunks of smaller size than the for-loop. Oops, I missed the __HAVE_ARCH_MEMCPY and was looking at the wrong memcpy implementation.. And in any case, I went totally off-track by your last comment. I hadn't realized that you were still referring to the way the code looks, rather than questioning the switch to memcpy. Sorry about that! I'll fix the odd-looking indentation and repost the patch. Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] netprio_cgroup: Use memcpy instead of the for-loop to copy priomap
From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com Date: Wed, 12 Sep 2012 13:54:05 +0530 On 09/12/2012 01:19 PM, David Miller wrote: From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com Date: Wed, 12 Sep 2012 11:37:47 +0530 + memcpy(new_priomap-priomap, old_priomap-priomap, + old_priomap-priomap_len * + sizeof(old_priomap-priomap[0])); This argument indentation is ridiculous. Try: memcpy(new_priomap-priomap, old_priomap-priomap, old_priomap-priomap_len * sizeof(old_priomap-priomap[0])); Using TABs exclusively for argumentat indentation is not the goal. Rather, lining the arguments up properly so that they sit at the first column after the first line's openning parenthesis is what you should be trying to achieve. OK, will fix it, thanks! And ignoring whatever stylistic convention we may or may not have, I find it impossibly hard to believe that the code quoted above looks good even to you. On second thoughts, I think the memcpy in this case will actually be worse since it will copy the contents in chunks of smaller size than the for-loop. I meant just coding style wise, not semantically. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
