Re: [Arpix-devel] New version of tarlib

[Simo S. <sim...@ti...>]

Some comments on the code:


Giovanni Corriga wrote:
> 
> Hi!
> 
> I'm sending a new version of tarlib that can be put in CVS tree. Now the
> source code is cleaner and safer.
> 
> The only high level function which is present is tl_getfilelist(). It
> does support only regular files (no long names yet) and directories. The
> only info you can get from archive is the name of each file, its type
> and its size.
> .tar.gz and .tar.bz2 files are not supported yet.
> 
> It is not much, but with this Gnomezip should be able to display .tar
> files' content.
> 
>         Bye
>                 Giovanni
> 
> PS. Hongli, you can add my name to the website now (Thanks!). BTW, can I
> have a CVS account, please?
> 

...snip...

>   ------------------------------------------------------------------------
> /*
>  *  tarlib.c
>  *
>  *  Tarlib: a library to handle .tar files
>  *  Copyright (C) 2000 Giovanni Corriga (Valkadesh)
>  *
>  *  Based upon the GNU tar program source code
>  *  GNU tar is copyright (C) 1999 Free Software Foundation
>  */
> 
> /*
...snip...
>   /* support functions */
> int check_file (char* file)
> {
>   union block block;
>   char* magic;
>   int retval = 0;
>   FILE* fp = fopen(file, "rb");

I think you should use standard file descriptors (open, write, read,
seek) and not file streams as it is more portable across systems.

> 
>   if (!fp)
>     return -1;
> 
>   if (fread(&block, sizeof(union block), 1, fp) != 1)
>   {
>     fclose(fp);
>     return -1;
>   }
> 
>   magic = block.header.magic;
> 
>   if (memcmp(magic, TMAGIC, sizeof(char)*TMAGLEN))
>     retval = 1; /* posix archive */
>   if (memcmp(magic, OLDGNU_MAGIC, sizeof(char)* 8))
>     retval = 2; /* GNU archive */

Using sizeof(char) in this contest is a bug, file format declares magic
beeing long 8 bytes!
This is indipendent from how many bytes a char use on the platform.
On platforms with chars of greater than 8bit size this would break as
they will copy more bytes causing possible memory leaks.


> 
>   fclose(fp);
> 
>   return retval;
> }
> 
...snip...


-- 
sim...@ti...
http://www.geocities.com/SiliconValley/9757