On Tuesday 24 June 2008 23:48, [EMAIL PROTECTED] wrote:
> Hello, Denys!
> 
> Made a new patch. Tried to fix the issues you pointed out.

+       do {
+               /* search for the first char in word */
+               ptr = memchr(ptr, *word, len - (ptr - (char*)the_module));
+               if (ptr == NULL) /* no occurance left, done */
+                       return NULL;
+               if (!strncmp(ptr, word, strlen(word))) {
+                       ptr += strlen(word);
+                       break;
+               }
+               ++ptr;
+       } while (1);

Can you move strlen outside of the loop?


+       fprintf(fp, "\n");

Nitpicking department says "fputc".


+static void process_module(char *modules_dep, char *name
+       USE_FEATURE_MODPROBE_NG_OPTIONS_ON_CMDLINE(, const char 
*cmdline_options))
+{
...
+                               process_module(modules_dep, s
+                                       
USE_FEATURE_MODPROBE_NG_OPTIONS_ON_CMDLINE(, cmdline_options)
+                               );

There is a neat trick how to make it readable. Put this before
the definition:

#if !ENABLE_FEATURE_MODPROBE_NG_OPTIONS_ON_CMDLINE
#define process_module(a, b, c) process_module(a, b)
#endif


+       // if depmod called ->
+       if ('d' == applet_name[0]) {
+               // force recreating modules.dep.bb
+               // N.B. to cope well-known "sendmail bug" we should ensure
+               // we are not writing to a symlink (to, e.g., /etc/passwd)
+               // placed by a malicious user in world-writable /tmp directory
+               // OTOH if we just unlink(DEPMOD_FILE) our depmod can kill smth 
unwillingly.
+               // So I prefer to bail out if depmod has to overwrite smth. 
Comments?
+               struct stat st;
+               if (!lstat(DEPMOD_FILE, &st)) {
+                       if (!(option_mask32 & OPT_q)) {
+                               // complain
+                               errno = EEXIST;
+                               bb_perror_msg_and_die(DEPMOD_FILE);
+                       } else
+                               goto quit;
+               }
+       }

I think depmod must create modules.dep.bb in /lib/modules, not in /tmp.
Just bail out if it can't create a file there.


+       if (!modules_dep) {
+               // depmod: dump modules definitions
+               // TODO: locking!!!
+               // TODO: locking!!!
+               // TODO: locking!!!
+               // TODO: locking!!!
+               // TODO: locking!!!
+               FILE *fp = fopen(DEPMOD_FILE, "w");
+               if (fp && recursive_action(".", 
+                       ACTION_RECURSE, /* flags */
+                       fileAction, /* file action */
+                       NULL, /* dir action */
+                       fp, /* user data */
+                       0) /* depth */
+               ) {
+                       fprintf(fp, "\n"); // finalize definitions
+                       fclose(fp);
+                       // depmod was called? -> we're done
+                       if ('d' == applet_name[0])
+                               return EXIT_SUCCESS;
+                       // read definitions
+                       modules_dep = xmalloc_xopen_read_close(DEPMOD_FILE, 
NULL);
+               }
+       }

Locking can be provided by atomic rename. Roughly:

        // we are in /lib/modules[/X.Y.Z]
        template = xstrdup(DEPMOD_FILE "XXXXXX");
        fd = mkstemp(template);
        fp = fdopen(fd, "w");
        generate_modprobe_dep_bb(fp);
        fclose(fp); // closes fd too
        rename(template, DEPMOD_FILE); <=== atomically replaces modules.dep.bb

> Please, try it.

Will do in a minute.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Reply via email to