[PATCH] fileread for the std VMOD

Poul-Henning Kamp phk at phk.freebsd.dk
Tue Nov 23 09:41:33 CET 2010


In message <AANLkTim=gN=WDqMNt-ngPqVj+W5QOLxKJa0Wd=N3FcYn at mail.gmail.com>, Sanj
oy Das writes:

>Here's an implementation which uses malloc instead of mmap (and hence
>bypasses both the issues altogether); and a more rigorous testcase.

Yes, that is a better solution.

Can I get you to put fileread into a separate source file ?  It is
sufficiently complex for that.

Some nits from FlexeLint:

../../lib/libvmod_std/vmod_std.c  288  Warning 529: Symbol 'aligned_sz' (line
    207) not subsequently referenced
../../lib/libvmod_std/vmod_std.c  207  Info 830: Location cited in prior
    message

(Use the autogen.des script to get developer settings and GCC would have
told you this)

../../lib/libvmod_std/vmod_std.c  288  Warning 550: Symbol 'pagesize' (line
    207) not accessed
../../lib/libvmod_std/vmod_std.c  207  Info 830: Location cited in prior
    message

        pthread_rwlock_unlock(&filelist_lock);
../../lib/libvmod_std/vmod_std.c  245  Warning 534: Ignoring return value of
    function 'pthread_rwlock_unlock(struct pthread_rwlock **)' (compare with
    line 234, file /usr/include/pthread.h, module cache_acceptor.c)
/usr/include/pthread.h  234  Info 830: Location cited in prior message
                       _
        fstat(fd, &buf);
../../lib/libvmod_std/vmod_std.c  250  Warning 534: Ignoring return value of
    function 'fstat(int, struct stat *)' (compare with line 307, file
    /usr/include/sys/stat.h, module mgt_param.c)
/usr/include/sys/stat.h  307  Info 830: Location cited in prior message

        ALLOC_OBJ(iter, CACHED_FILE_MAGIC);
>> missing assert check here

        iter->file_name = strdup(file_name);

        close(fd);
../../lib/libvmod_std/vmod_std.c  278  Warning 534: Ignoring return value of
    function 'close(int)' (compare with line 324, file /usr/include/unistd.h,
    module cache_acceptor.c)
/usr/include/unistd.h  324  Info 830: Location cited in prior message

(Put an AZ() on it, We are _very_ paranoid about fd-usage in varnish)

+Info 765: external 'filelist_lock' (line 198, file ../../lib/libvmod_std/vmod_s
td.c)
+    could be made static
+../../lib/libvmod_std/vmod_std.c  198  Info 830: Location cited in prior
+    message
+Info 765: external 'filelist_update' (line 199, file ../../lib/libvmod_std/vmod
_std.c)
+    could be made static
+../../lib/libvmod_std/vmod_std.c  199  Info 830: Location cited in prior
+    message
+Info 843: Variable 'filelist_update' (line 199, file ../../lib/libvmod_std/vmod
_std.c)
+    could be declared as const
+../../lib/libvmod_std/vmod_std.c  199  Info 830: Location cited in prior
+    message




-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.




More information about the varnish-dev mailing list