Re: [msysGit] Re: [PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Hi Junio, On Wed, 8 Oct 2014, Junio C Hamano wrote: Marat Radchenko ma...@slonopotamus.org writes: #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) 1024 * 1024 * 8192 overflows 32-bit unsigned, but is size_t always large enough? Just checking. The diff is a bit misleading as to what it *actually* changes. It *just* casts the result to size_t. The arithmetic is performed with longs (thanks to the l in 1024l) and it only overflows 32 bit iff the sizeof() test verifies that we're at least on 64 bit -- this arithmetic operation is the same as before the patch. I was fooled by the diff myself (adding another parenthesis just to add the cast would probably have helped, though). IMHO this is a good demonstration how a commit message that goes slightly beyond the necessary can help tons of time by avoiding to let every reviewer/reader go through the exact same steps of puzzlement. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Marat Radchenko ma...@slonopotamus.org writes: On Wed, Oct 08, 2014 at 01:02:10PM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Marat Radchenko ma...@slonopotamus.org writes: Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index b338277..101c9d7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) 1024 * 1024 * 8192 overflows 32-bit unsigned, but is size_t always large enough? Just checking. Heh, I was being silly. This gives the default value for a variable whose type is size_t, so it would better fit. So please throw 13 in the list of changes I found sensible in the other message. Is it an Acked-by? Not really. It just shows that the change needs to be explained well in the proposed log message. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index b338277..101c9d7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) #ifdef NO_PREAD #define pread git_pread -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Marat Radchenko ma...@slonopotamus.org writes: Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index b338277..101c9d7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) 1024 * 1024 * 8192 overflows 32-bit unsigned, but is size_t always large enough? Just checking. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Junio C Hamano gits...@pobox.com writes: Marat Radchenko ma...@slonopotamus.org writes: Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index b338277..101c9d7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ -((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) +((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) 1024 * 1024 * 8192 overflows 32-bit unsigned, but is size_t always large enough? Just checking. Heh, I was being silly. This gives the default value for a variable whose type is size_t, so it would better fit. So please throw 13 in the list of changes I found sensible in the other message. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
On Wed, Oct 08, 2014 at 01:02:10PM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Marat Radchenko ma...@slonopotamus.org writes: Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index b338277..101c9d7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) 1024 * 1024 * 8192 overflows 32-bit unsigned, but is size_t always large enough? Just checking. Heh, I was being silly. This gives the default value for a variable whose type is size_t, so it would better fit. So please throw 13 in the list of changes I found sensible in the other message. Is it an Acked-by? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index 9d2d5ab..5f6659c 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) #ifdef NO_PREAD #define pread git_pread -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/14] git-compat-util.h: fix integer overflow on IL32P64 systems
Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index 9d2d5ab..5f6659c 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -474,7 +474,7 @@ extern int git_munmap(void *start, size_t length); #endif #define DEFAULT_PACKED_GIT_LIMIT \ - ((1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) + ((size_t)(1024L * 1024L) * (sizeof(void*) = 8 ? 8192 : 256)) #ifdef NO_PREAD #define pread git_pread -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
