HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Subset of UNIX standard tar implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unixstandardimplementationsubsettar

Problem

I recently implemented a subset of the ustar archiving utility as my first year programming one project. I would like to get it reviewed and if anyone has anything to point out.

dir.c

```
#include
#include
#include
#include
#include
#include
#include
#include "star.h"
void dir_untar(struct header_ustar *h)
{
mode_t mode;
char modeoct[9] = {0};
strncpy(modeoct,h->mode,8);
sscanf(modeoct,"%o",&mode);
char filename[260]={0};
if(h->prefix[0]){
strncat(filename,h->prefix,155);
strcat(filename,"/");
}
strncat(filename,h->name,100);
mkdir(filename,mode);
}
void dir_tar(FILE tarf, char dirname, struct stat *s)
{
struct header_ustar h;
bzero(&h,sizeof(struct header_ustar));
int len = (int)strlen(dirname);
if(len>255)
return;
if(len>100){
int j=154;
while(dirname[j]!='/')
j--;
if(len-j>100)
return;
strncpy(h.prefix,dirname,(size_t)j);
strncpy(h.name,dirname+j+1,(size_t)(len-j-1));
}else
strcpy(h.name,dirname);
char buf=malloc(sizeof(char)512);
sprintf(buf,"%08o",s->st_uid);
strncpy(h.uid,buf,8);
sprintf(buf,"%08o",s->st_gid);
strncpy(h.gid,buf,8);
sprintf(buf,"%012o",0);
strncpy(h.size,buf,12);
sprintf(buf,"%012o",(int)s->st_mtime);
strncpy(h.mtime,buf,12);
h.typeflag = '5';
sprintf(buf,"%08o",s->st_mode);
strncpy(h.mode,buf,8);
strcpy(h.magic,MAGIC);
strncpy(h.version,"00",2);
unsigned int chksum = checksum(&h);
sprintf(h.checksum,"%06o",chksum);
h.checksum[7] = ' ';
fwrite(&h,sizeof(struct header_ustar),1,tarf);

DIR *dp;
dp = opendir(dirname);

struct dirent * content;
while((content= readdir(dp)) != NULL)
{
if(strcmp(content->d_name,"..")==0 || strcmp(content->d_name,".")==0)
continue;
struct stat s2;
char filename[255]={0};
strcat(filename,dirname);
int l2 = (

Solution

This line: * \breif reads the file from tar archive and write it

contains a misspelling of the key word brief

For readability/understandability, separate code blocks: (for, if, else, while, do...while, switch, case, default) by a blank line

when calling system functions: fopen() and fread() always check the returned value to assure the operation was successful

the switch() statements should include a default: case so unexpected input, etc is caught and handled

in the file: untar.c the controlling variable for the while() loop should be the call to fread(), and have isvalid() be in an if statement inside the loop, Then the loop will be properly exited when the fread() fails.

The code contains several 'magic' numbers. 'magic' numbers make the code difficult to understand, debug, maintain. Suggest using #defines or an enum to give those magic numbers meaningful names then use those meaningful names throughout the code. Some of the 'magic' numbers are 100, 256, 154.

The stat() function needs a path for the first parameter, not just the file name.

regarding the header file: star.h leading underscores, especially double leading underscores are 'effectively' reserved for the compiler. Strongly suggest removing the double leading underscores and use the complete file name in the #ifndef, etc statements. I.E:

#ifndef STAR_H
#define STAR_H
...
#endif


when calling malloc(), always check (!=NULL) the returned value to assure the operation was successful.

regarding this line: long int toseek = 512-ftell(tarf)%512;

the % operator has a lower precedence than the - operator so the result of the line will be drastically different that what is expected.

Also, if the file is less than 512 bytes long, then the % operator will threat the resulting negative value as a very large positive value, again resulting a drastically different value than what is expected.

Code Snippets

#ifndef STAR_H
#define STAR_H
...
#endif

Context

StackExchange Code Review Q#115575, answer score: 2

Revisions (0)

No revisions yet.