Re: chfs support in makefs

[christos%astron.com@localhost (Christos Zoulas)]

In article <op.wcuzrso7bcua1o@localhost.localdomain>,
Tamas Toth <ttoth%inf.u-szeged.hu@localhost> wrote:
>-=-=-=-=-=-
>
>Thanks for the comments. I fixed the problems.
>

Looks good, some more stuff and commit it, don't wait for another round.
We'll fix the remaining stuff in the tree if there are any left.

Best,

christos

+       if ((var = strdup(option)) == NULL) {
+               err(1, "Allocating memory for copy of option string");

change all the err{,x}(1, -> err{,x}(EXIT_FAILURE,

+static int
+chfs_create_image(const char *image, fsinfo_t *fsopts)
+{
+       assert (image != NULL);
+       assert (fsopts != NULL);

No space before paren like everywhere else.

+               if (snprintf(path, sizeof(path), "%s/%s", dir, cur->name)
+                       >= sizeof(path)) {
+                       errx(1, "Pathname too long.");

Continuation lines are indented by 4, and errors generally don't have periods.

+static void
+buf_write(fsinfo_t *fsopts, const void *buf, size_t len)
+{
+       ssize_t retval;
+       const char *charbuf = buf;
+
+       while (len > 0) {
+               retval = write(fsopts->fd, charbuf, len);
+
+               if (retval == -1) {
+                       err(1, "ERROR while writing\n");
+               }

No \n in err(), it puts one itself.

+
+               len -= retval;
+               charbuf += retval;
+               img_ofs += retval;
+       }
+}
+
+void
+padblock(fsinfo_t *fsopts)
+{
+       chfs_opt_t *chfs_opts = fsopts->fs_specific;
+
+       while (img_ofs % chfs_opts->eraseblock) {
+               buf_write(fsopts, ffbuf, MIN(sizeof(ffbuf),
+                       chfs_opts->eraseblock - (img_ofs % 
chfs_opts->eraseblock)));

continuation lines are 4 indented.
+       }
+}
+
+static void
+padword(fsinfo_t *fsopts)
+{
+       if (img_ofs % 4) {
+               buf_write(fsopts, ffbuf, 4 - img_ofs % 4);
+       }
+}
+
+static void
+pad_block_if_less_than(fsinfo_t *fsopts, int req)
+{
+       chfs_opt_t *chfs_opts = fsopts->fs_specific;
+       if ((img_ofs % chfs_opts->eraseblock) + req > chfs_opts->eraseblock) {
+               padblock(fsopts);

IBID, and I am not reporting anymore.

+
+void
+write_file(fsinfo_t *fsopts, fsnode *node, const char *dir)
+{
+       int fd;
+       ssize_t len;
+       char *name = node->name;
+       chfs_opt_t *chfs_opts;
+       unsigned char *buf;
+       uint32_t fileofs = 0;
+
+       chfs_opts = fsopts->fs_specific;
+       buf = malloc(chfs_opts->pagesize);

check malloc, at least assert()

+       
+       if (node->type == S_IFREG || node->type == S_IFSOCK) {
+               char *longname = malloc((strlen(dir) + strlen(name)) * 
sizeof(char));

No need to allocate for asprintf(), it does it for you.

+               asprintf(&longname, "%s/%s", dir, name);
+
+               fd = open(longname, O_RDONLY, 0444);
+
+               while ((len = read(fd, buf, chfs_opts->pagesize))) {
+                       if (len < 0) {
+                               warn("ERROR while reading %s\n", longname);

no \n in warn.

+                               free(buf);
+                               close(fd);
+                               return;
+                       }
+
+                       write_data(fsopts, node, buf, len, fileofs);
+                       fileofs += len;
+               }
+               close(fd);      
+       fdata.data_length = htole32(len);
+       fdata.offset = htole32(ofs);
+       fdata.data_crc = htole32(crc32(0, (uint8_t *) buf, len));

no space after cast like everywhere else.

+       fdata.node_crc = htole32(crc32(0, (uint8_t *)&fdata, sizeof(fdata) - 
4));