CODEWITHSUNDEEP
cunixgcc
user1067334
user1067334

Reputation: 243

What might be wrong with this C in Unix code

I am fairly new to writing C code and I am sure I have something wrong at the very basic level. I am running a small code to get the attributes of a file and this function below returns those attributes. char *path would contain something like "/home/etc/bin"

/* copy the attributes into a character pointer */
unsigned char * copyAttributes (char *path)
{
    struct stat buf;
    stat (path, &buf);
    int nMalloc = sizeof(dev_t) + sizeof(ino_t) + sizeof(mode_t)+ sizeof(nlink_t)+ sizeof(uid_t)+ sizeof(gid_t)+ sizeof(dev_t)+ sizeof(off_t)+ sizeof(blksize_t)+ sizeof(blkcnt_t)+ sizeof(time_t)+ sizeof(time_t)+ sizeof(time_t)+ 1;
    char *pathForStatBuff = malloc(nMalloc);
    printf("%d\n",nMalloc);
    unsigned long base = 0;
    memcpy(pathForStatBuff + base,&buf.st_dev,sizeof(dev_t));
    base = base + sizeof(dev_t);    
    memcpy(pathForStatBuff + base,&buf.st_ino,sizeof(ino_t));
    base = base + sizeof(ino_t);
    memcpy(pathForStatBuff + base,&buf.st_mode,sizeof(mode_t));
    base = base + sizeof(mode_t);
    memcpy(pathForStatBuff + base,&buf.st_nlink,sizeof(nlink_t));
    base = base + sizeof(nlink_t);
    memcpy(pathForStatBuff + base,&buf.st_uid,sizeof(uid_t));
    base = base + sizeof(uid_t);
    memcpy(pathForStatBuff + base,&buf.st_gid,sizeof(gid_t));
    base = base + sizeof(gid_t);
    memcpy(pathForStatBuff + base,&buf.st_rdev,sizeof(dev_t));
    base = base + sizeof(dev_t);
    memcpy(pathForStatBuff + base,&buf.st_size,sizeof(off_t));
    base = base + sizeof(off_t);
    memcpy(pathForStatBuff + base,&buf.st_blksize,sizeof(blksize_t));
    base = base + sizeof(blksize_t);
    memcpy(pathForStatBuff + base,&buf.st_blocks,sizeof(blkcnt_t));
    base = base + sizeof(blkcnt_t);
    memcpy(pathForStatBuff + base,&buf.st_atime,sizeof(time_t));
    base = base + sizeof(time_t);
    memcpy(pathForStatBuff + base,&buf.st_mtime,sizeof(time_t));
    base = base + sizeof(time_t);
    memcpy(pathForStatBuff + base,&buf.st_ctime,sizeof(time_t));
    base = base + sizeof(time_t);   

    printf("Printing pathForStatBuff = %s\n",pathForStatBuff);
    return pathForStatBuff;
}

Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p 53 Printing pathForStatBuff = p

This is the output I always get, I can't seem to figure out what I am doing wrong. Could you guys guide me on what it might be. Thanks.

Upvotes: 0

Views: 525

Answers (4)

gbulmer
gbulmer

Reputation: 4290

At first sight, the code:

int nMalloc = sizeof(dev_t)  + sizeof(ino_t) + sizeof(mode_t)+ ...

is doing exactly the same as 

int nMalloc = sizeof(struct stat);

Edit: WARNING - caf pointed out that the 

int nMalloc = sizeof(dev_t)  + sizeof(ino_t) + ...

is not necessarily exactly the same as int nMalloc = sizeof(struct stat);

The fields within struct stat might have alignment constraints which cause there to be 'holes' or padding in between fields. There might also be undocumented fields. So struct stat might be bigger than the sum of its published fields.

This would be straightforward to check.

if (nMalloc != sizeof(struct stat)) { fprintf(stderr, "...\n");

but might change in future, so your original is more robust.

Based on caf's proposition, this is also not necessarily identical: 

unsigned long base = 0;
memcpy(pathForStatBuff + base,&buf.st_dev,sizeof(dev_t));
base = base + sizeof(dev_t);    
...
memcpy(pathForStatBuff + base,&buf.st_ctime,sizeof(time_t));
base = base + sizeof(time_t);

to: memcpy(pathForStatBuff, &buf, sizeof(struct stat));

but the size test would detect that.

So, field assignment to a new struct would allow you to get control of the fields needed, their order, and some aspects of layout (same compiler/same platform).

If memcpy is preferred, look at mempcpy. It is like memcpy, but returns a pointer to the byte after the last one, so you don't need to do:

p = memcpy(pathForStatBuf  + base, ...
base = base + sizeof(...)
...
base = base + sizeof(...)
p = memcpy(pathForStatBuf  + base, ...
base = base + sizeof(...)

but instead have an extra pointer and do 

p = memcpy(p, ...
p = memcpy(p, ...

Do you want to get a printable form? If that is the case, you will need to use sprintf, printf, or fprintf. As you are learning, I would convert all of the memcpy's to printf("..., buf....field ...); and see them immediately, that will be easier than getting a string right first time.

Upvotes: 0

caf
caf

Reputation: 239041

You are copying the data into your memory block fine. The only problem is that you can't use printf with %s to show the result, because it's not a null terminated string (or a string of any kind).

If you want to print out the contents of the memory block for debugging purposes, you could print it in hex:

{
    int i;

    printf("Printing pathForStatBuff = { ");
    for (i = 0; i < sizeof nMalloc; i++)
        printf("%#.2x ", (unsigned char)pathForStatBuff[i]);
    printf("}\n");
}

Upvotes: 1

Guido
Guido

Reputation: 2634

You're copying the data with no transformation what-so-ever, so if buf.st_dev turned out to have a least significant byte of 0 (I'm assuming little endian) you'll end up with an empty string, from your example it looks like the LSB is always 'p' (113). You could try using a function to which you give the string along with it's length and it prints it in hexadecimal, this way you could see the data. You need to keep the difference between a number and it's decimal representation in mind, the number 231 has no similarity to the string "231" Otherwise you could do something like:

sprintf(pathForStatBuf, "Dev: %d\nIno: %d\n....", buf.st_dev, buf.st_ino, ...);

The downside to that is that the buffer size is not easy to define.

I guess you're only doing this to learn about stat and stuff, so you could just allocate a big string (2048 chars or so) and you should stay within bounds.

Upvotes: 3

Some programmer dude
Some programmer dude

Reputation: 409176

You are trying to use binary data as a string. In C strings ends with a zero, which means that if one member is zero then when you copy it into the buffer it will be marking the end of the string.

If you want to convert all data to a string you should use e.g. snprintf instead:

char buffer[128];
snprintf(buffer, sizeof(buffer), "%d %d %d",
         buf.st_dev, buf.st_ino, buf.st_mode);

See the manual page for snprintf for more information about the function.

(In my example I only used three members of the structure, add all you need.)

Upvotes: 3

Related Questions