Re: Listview updates (G0)

[Paul Rupe]

On 27 Sep 2002, [e-mail] wrote:
> @@ -3564,7 +3604,6 @@
>              dis.rcItem.bottom = dis.rcItem.top +
>              infoPtr->nItemHeight; OffsetRect(&dis.rcItem, ptOrig.x,
>              0);
> -
>        ZeroMemory(&item, sizeof(item));
>        item.iItem = nItem;
>     item.iSubItem = 0;
>        item.mask = LVIF_PARAM;
> @@ -5555,7 +5584,6 @@
>    {
>      LVITEMW lvItem;
> - ZeroMemory(&lvItem, sizeof(lvItem));
> lvItem.mask = LVIF_INDENT;
> lvItem.iItem = nItem;
> lvItem.iSubItem = 0;

With these two ZeroMemory calls removed, I'm getting crashes in Xnews due 
to the uninitialized pszText member.  In both cases, it happens when 
LISTVIEW_GetItemT gets the LVITEMW structure and calls notify_dispinfoT 
(line 5186 or so).

Listview updates (G0)

[Dimitrie O. Paun]

On September 28, 2002 03:19 pm, Paul Rupe wrote:
> With these two ZeroMemory calls removed, I'm getting crashes in Xnews due
> to the uninitialized pszText member.  In both cases, it happens when
> LISTVIEW_GetItemT gets the LVITEMW structure and calls notify_dispinfoT

Which means that notify_dispinfoT is broken. This way we find the bug,
which is my point exactly against adding this zero memory calls. Not only
they bloat the code, making it slower, harder to read, etc., but most
importantly they hide exactly this types of bugs. We can not go "easy" on
our functions by zeroing the memory first, because the exact same functions
will be called by app code, and there we have no guarantees that the app
will clear the memory before hand. We have to make our functions work 
properly in the most harsh, documented environment.

Thanks for spotting it. Here's the fix (relative to H0):

--- dlls/comctl32/listview.c.H0 Fri Sep 27 15:58:32 2002
+++ dlls/comctl32/listview.c    Sat Sep 28 16:43:12 2002
@@ -601,7 +601,7 @@
   else
     realNotifCode = notificationCode;

-  if (is_textT(pdi->item.pszText, isW))
+  if ((pdi->item.mask & LVIF_TEXT) && is_textT(pdi->item.pszText, isW))
   {
     if (isW && infoPtr->notifyFormat == NFR_ANSI)
         convertToAnsi = TRUE;

Listview updates (G0)

[Sylvain Petreolle]

Thus can we consider the result of :
grep ZeroMemory `find . -name '*.c'` and say "there are probably bugs
here " ?
> Which means that notify_dispinfoT is broken. This way we find the
> bug,
> which is my point exactly against adding this zero memory calls. Not
> only
> they bloat the code, making it slower, harder to read, etc., but most
> importantly they hide exactly this types of bugs. We can not go

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

Listview updates (G0)

[Dimitrie O. Paun]

On September 29, 2002 03:42 pm, Sylvain Petreolle wrote:
> Thus can we consider the result of :
> grep ZeroMemory `find . -name '*.c'` and say "there are probably bugs
> here " ?

I would like to say that that's the case, but I'm a little worried
that will not hold for notification messages. You see, notifications
messages are special in that they go *to* the application, that is,
to code we do not control. And if that code breaks because we push it
to the limit, there's nothing we can do. So I think it's better to
follow the network protocol: be very strict in what we send, and very
liberal in what we receive.