[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] cpio and tar filesystem
From: |
Robert Millan |
Subject: |
Re: [PATCH] cpio and tar filesystem |
Date: |
Sun, 23 Dec 2007 22:48:51 +0100 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
On Mon, Dec 24, 2007 at 04:04:31AM +0800, Bean wrote:
>
> * conf/i386-pc.rmk (grub_emu_SOURCES): Add cpio.c.
>
> * conf/i386-efi.rmk (grub_emu_SOURCES): Likewise.
>
> * conf/i386-linuxbios.rmk (grub_emu_SOURCES): Likewise.
>
> * conf/powerpc-ieee1275.rmk (grub_emu_SOURCES): Likewise.
Perhaps it'd be a good idea to move the arch-independant part of
grub_emu_SOURCES to common.rmk (as grub_emu_SOURCES += foo), to avoid
having to update all the files so often ...
> diff --git a/disk/loopback.c b/disk/loopback.c
> index 9d48def..31d8116 100644
> --- a/disk/loopback.c
> +++ b/disk/loopback.c
> @@ -214,7 +214,7 @@ grub_loopback_read (grub_disk_t disk,
> grub_disk_addr_t sector,
> if (pos > file->size)
> {
> grub_size_t amount = pos - file->size;
> - grub_memset (buf + size - amount, 0, amount);
> + grub_memset (buf + (size << GRUB_DISK_SECTOR_BITS) - amount, 0,
> amount);
Ugh :-) I check this in right now.
> +struct HEAD_USTAR
> +{
> + char name[100];
> + char mode[8];
> + char uid[8];
> + char gid[8];
> + char size[12];
> + char mtime[12];
> + char chksum[8];
> + char typeflag;
> + char linkname[100];
> + char magic[6];
> + char version[2];
> + char uname[32];
> + char gname[32];
> + char devmajor[8];
> + char devminor[8];
> + char prefix[155];
These tabs should be spaces.
> + if (grub_disk_read (data->disk, 0, data->hofs, sizeof(hd), (char*)&hd))
> [...]
> + if (grub_disk_read (data->disk, 0, data->hofs, sizeof(hd), (char*)&hd))
`(char *) &hd' here (same for all other casts).
Btw, this line seems to be the same on both cases. It can be moved out of
`if (data->mode == MODE_BCPIO)' to save some space?
> + if (hd.namesize & 1)
> + hd.namesize++;
> [...]
> + if (data->size & 1)
> + (*ofs)++;
I find this confusing. AFAICT `hd.namesize == 1' would archieve the same and
seems to be more consistent with your use of this variable as a counter.
> + if ((*name = grub_malloc (hd.namesize))==NULL)
> [...]
> + if ((*name = grub_strdup (hd.name))==NULL)
Please add some spaces: ` == NULL'
> + if (grub_memcmp(hd.magic, MAGIC_USTAR, sizeof(MAGIC_USTAR) - 1))
> [...]
> + data->size = grub_strtoul(hd.size, NULL, 8);
`grub_memcmp (', `grub_strtoul (', etc. Same for other function calls.
> + return (grub_disk_read (data->disk, 0, data->dofs + file->offset,
> + len, buf))?-1:len;
` ? -1 : len'
> +#ifndef GRUB_UTIL
> + grub_dl_unref (my_mod);
> +#endif
> [...]
> +#ifndef GRUB_UTIL
> + my_mod = mod;
> +#endif
Are you sure these are still needed? We have a few modules that use them but
AFAIK are not necessary at this time.
--
Robert Millan
<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
Re: [PATCH] cpio and tar filesystem, Robert Millan, 2007/12/23