Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
On Wed, Mar 21, 2018 at 05:32:40PM +0800, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c index 6231070240..99a1856bfe 100644 --- a/dlls/comctl32/datetime.c +++ b/dlls/comctl32/datetime.c @@ -1477,6 +1477,7 @@ static BOOL DATETIME_GetIdealSize(DATETIME_INFO *infoPtr, SIZE *size) /* Get text font width */ for (i = 0; i < infoPtr->nrFields; i++) {
width = 0; DATETIME_ReturnFieldWidth(infoPtr, hdc, i, &width); size->cx += width;
Either DATETIME_ReturnFieldWidth() should zero-init *width, or it should return a BOOL to indicate success which should be checked by the callers.
Huw.
On 3/23/2018 2:35 PM, Huw Davies wrote:
On Wed, Mar 21, 2018 at 05:32:40PM +0800, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c index 6231070240..99a1856bfe 100644 --- a/dlls/comctl32/datetime.c +++ b/dlls/comctl32/datetime.c @@ -1477,6 +1477,7 @@ static BOOL DATETIME_GetIdealSize(DATETIME_INFO *infoPtr, SIZE *size) /* Get text font width */ for (i = 0; i < infoPtr->nrFields; i++) {
width = 0; DATETIME_ReturnFieldWidth(infoPtr, hdc, i, &width); size->cx += width;Either DATETIME_ReturnFieldWidth() should zero-init *width, or it should return a BOOL to indicate success which should be checked by the callers.
Another option is to return width as a return value. Having error status return seems like an overkill to me. Also I'd remove this block entirely:
--- TRACE ("%d,%d\n", infoPtr->nrFields, count); if (count>infoPtr->nrFields || count < 0) { WARN ("buffer overrun, have %d want %d\n", infoPtr->nrFields, count); return; } ---
Argument validation is redundant, and trace is not very useful.
Huw.
On Fri, Mar 23, 2018 at 02:42:50PM +0300, Nikolay Sivov wrote:
On 3/23/2018 2:35 PM, Huw Davies wrote:
On Wed, Mar 21, 2018 at 05:32:40PM +0800, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c index 6231070240..99a1856bfe 100644 --- a/dlls/comctl32/datetime.c +++ b/dlls/comctl32/datetime.c @@ -1477,6 +1477,7 @@ static BOOL DATETIME_GetIdealSize(DATETIME_INFO *infoPtr, SIZE *size) /* Get text font width */ for (i = 0; i < infoPtr->nrFields; i++) {
width = 0; DATETIME_ReturnFieldWidth(infoPtr, hdc, i, &width); size->cx += width;Either DATETIME_ReturnFieldWidth() should zero-init *width, or it should return a BOOL to indicate success which should be checked by the callers.
Another option is to return width as a return value. Having error status return seems like an overkill to me.
Yes, I just realised that too :)
Right, I should use width as return value and remove the redundant argument check.
On Fri 3 23 19:42, Nikolay Sivov wrote:
On 3/23/2018 2:35 PM, Huw Davies wrote:
On Wed, Mar 21, 2018 at 05:32:40PM +0800, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c index 6231070240..99a1856bfe 100644 --- a/dlls/comctl32/datetime.c +++ b/dlls/comctl32/datetime.c @@ -1477,6 +1477,7 @@ static BOOL DATETIME_GetIdealSize(DATETIME_INFO *infoPtr, SIZE *size) /* Get text font width */ for (i = 0; i < infoPtr->nrFields; i++) {
width = 0; DATETIME_ReturnFieldWidth(infoPtr, hdc, i, &width); size->cx += width; }Either DATETIME_ReturnFieldWidth() should zero-init *width, or it should return a BOOL to indicate success which should be checked by the callers.
Another option is to return width as a return value. Having error status return seems like an overkill to me. Also I'd remove this block entirely:
TRACE ("%d,%d\n", infoPtr->nrFields, count); if (count>infoPtr->nrFields || count < 0) {WARN ("buffer overrun, have %d want %d\n", infoPtr->nrFields, count); return; }
Argument validation is redundant, and trace is not very useful.
Huw.
On Fri, Mar 23, 2018 at 11:35:52AM +0000, Huw Davies wrote:
On Wed, Mar 21, 2018 at 05:32:40PM +0800, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/comctl32/datetime.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c index 6231070240..99a1856bfe 100644 --- a/dlls/comctl32/datetime.c +++ b/dlls/comctl32/datetime.c @@ -1477,6 +1477,7 @@ static BOOL DATETIME_GetIdealSize(DATETIME_INFO *infoPtr, SIZE *size) /* Get text font width */ for (i = 0; i < infoPtr->nrFields; i++) {
width = 0; DATETIME_ReturnFieldWidth(infoPtr, hdc, i, &width); size->cx += width;Either DATETIME_ReturnFieldWidth() should zero-init *width, or it should return a BOOL to indicate success which should be checked by the callers.
Actually, if you don't need the error return, the function could simply return the width.
Huw.
-
Huw Davies -
Nikolay Sivov -
Zhiyi Zhang