Hi Chris, hi to all,

Thanks a lot for your hard work on Polipo.  Here's my review of the
latest commits, in no particular order.


1. "Add an extra sanity check to avoid memmove segfault"

Could you please clarify?  I don't understand this fix.  Both reqlen and
reqbegin are signed, so it's not a question of sign contagion, right?


2. "Remove dead store in..."

I'm certainly not a big fan of these changes.  There are good reasons
for these stores, even though clang detects them as dead.

My policy is that I never write

  free(p);

but instead write

  free(p);
  p = NULL;

even when I'm not going to use p afterwards.

The reason for that is that in the former case, if I use p afterwards
I'm going to get a mysterious malloc arena corruption.  With the NULL
assignment, I'm guaranteed to get a reliable crash.

Similarly, I do

  close(fd);
  fd = -1;

so that the next access through fd gets EBADF.

I strongly recommend reverting this series of patches.


3. "Don't crash when parsing a malformed Cache-Control: header."

Yep, definitely my bug.  Thanks for fixing this.

If you have the time -- perhaps using strtol and proper error checking
might be a good idea?


4. "Fix what looks like a variable mixup in makeDiskEntry()."

Not an urgent task, but I suggest drastically simplifying this whole
mess.

I originally wrote the disk cache code on the assumption that people
would use read-only caches -- think a cache on CD, or a read-only NFS
mount shared between multiple Polipo instances.  As far as I'm aware,
nobody has ever used this feature, so I'd recommend dumping it.

Outline: remove the writeable flag from disk cache entries.  In
makeDiskEntry, only try to open with O_RDWR, and fail if that fails.

In doing that, be careful to preserve the lock-less protocol for
updating the cache concurrently -- see Section 4.3.4 in the manual.


I think that's all for now.

                                        Juliusz

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
Polipo-users mailing list
Polipo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/polipo-users

Reply via email to