Hoi.
[2020-03-21 18:43] Philipp Takacs <phil...@bureaucracy.de>
> [2020-03-21 10:05] markus schnalke <mei...@marmaro.de>
> > [2020-03-19 12:33] Philipp Takacs <phil...@bureaucracy.de>
> > > I have also removed the
> > > clear_msg_flags loop from folder_read and folder_realloc. In most cases
> > > we use calloc, so this isn't necesarry. At one place in folder_realloc
> > > realloc is used. There I have add a memset.
> > >
> > > comment?
> >
> > Why were (in the section down below) only the flags outside the
> > current message range cleared? Now you clear all flags.
>
> Not sure whats make you think this. But if I had to guess I would
> say it's the "- lo".
No, it's the comment a bit more down in the code:
> > - /*
> > - ** Clear all the flags for entries outside
> > - ** the current message range for this folder.
> > - */
Maybe I was irritated that you removed this comment but still
performed a similar action. The memset() line is pretty complex
to understand. You could add a similar two-line comment to it,
explaining what you explained to me here (only a bit more
destilled):
> So a shourt exircourse to the msgstats. msgstats
> is an array of the lenght ``mp->hghoff - mp->lowoff''. lowoff is the
> starting offset, so if you want the stats of message 5 you access
> ``mp->msgstats[5 - mp->mp->lowoff]. hghoff is the end offset so you
> can change flags for all messagestats in the range lowoff <= i <= hghoff.
> folder_realloc in this code path increases the hghoff. Now I have to
> clear all data after the old hghoff. by the way there was a off by one
> in my path. I fixed it now it looks like this:
>
> > > memset(mp->msgstats + mp->hghoff - lo + 1, 0, hi - mp->hghoff);
>
> This actually clears less then the code before. The code before
> started at ``mp->hghmsg + 1''. Now it starts hghoff which is
> greater or equal to hghmsg.
>
> > Does that change the behavior?
>
> Yes, like the pathes for seq_read it don't clears flags for msgs
> which it belive don't exists. But this is actually the reason
> I created this patch.
Alright. That makes sense.
> > Or, if your memset() clears only specific
> > parts, you probably should put a comment there to explain it, as
> > this might not be what the programmer usually expects from a
> > memset() after a (re)alloc().
>
> To me this looks completly like this what I would expect.
Of course; you've written the code. ;-)
> But I'll think about a good comment.
Is what you actually clear only the newly allocated memory? If so
then you could write something like:
/* zero the newly allocated memory, i.e. no flags set */
Or:
/* clear the newly allocated msg flag space */
(I really need to look at the code, not only at the diffs and the
mails ...)
meillo
