The code reads the specified number of bytes from a specified offset in a file, I have handled the partial reads and bytes> buf_size conditions. Could anyone review the code? and say whether the handling strategy is good. The constraint I am not allowed to use system calls other than open, close, read, write,Thanks a ton.
#include<stdio.h>
#include<stdlib.h>
#include<unistd.h>
#include<sys/types.h>
#include<error.h>
#include<errno.h>
#include<fcntl.h>
#define buf_size 5
int main(int argc, char *argv[])
{
int bytes;
int offset;
int fd;
char *file;
char buf[buf_size];
int rlen = 0;
int len = 0;
int i = 0;
if (argc != 4)
error(1, 0, "Too many or less than the number of arguments");
file = argv[1];
offset = atoi(argv[2]);
bytes = atoi(argv[3]);
fd = open(file, O_RDONLY);
printf("The values of the file descriptor is : %d\n", fd);
if (fd == -1)
error(1, errno, "Error while opening the file\n");
while (1) {
rlen = read(fd, buf, offset);
if (rlen == -1)
error(1, errno, "Error in reading the file\n");
len = len + rlen;
if(len == offset) {
len = 0;
while (1) {
rlen = read(fd, buf, bytes);
if (rlen == -1)
error(1, errno, "Error in reading the file\n");
if (rlen == 0)
return 0;
len = len + rlen;
for (i = 0; i < rlen; i++)
putchar(buf[i]);
if (len == bytes) {
printf("\nSuccess\n");
return 0;
}
bytes = bytes - rlen;
}
}
}
}
```
1 Answer 1
Fails with large offset, bytes
read(fd, buf, offset);
reads out of buf[]
bounds when offset > buf_size
. Same for bytes
. This should be tested.
Unclear why buf[buf_size]
is so small (5). How about 4096 or 1 Meg?
Handle large offset
I'd expect code to handle offsets far larger than the sizeof buf
. Looping on read(fd, buf, offset);
if needed.
int
math
Files sizes can well exceed INT_MAX
. For offset, bytes
, I'd use long long
.
Unneeded loop
Rather than a for()
loop for (i = 0; i < rlen; i++) putchar(buf[i]);
, use write(1, ...)
.
Missing close()
Function Limitations
OP has "not allowed to use system calls other than open, close, read, write". What about error()
, printf()
, atoi()
, putchar()
- or are those classified differently?
With such a limit, looks like a lot of includes
.
Algorithm flaw?
The first while (1)
loop looks wrong.
Either the first iteration will get the expect offset
number of bytes and proceed to the 2nd while ()
.
Or
it will read insufficient number of bytes (for reasons other than no -more bytes will ever exist) and loop again, trying to read offset
bytes again. I'd expect that offset
would have been reduced by the previous rlen
. Without that reduction, if(len == offset)
may never be satisfied as reading too many offset
bytes is then possible.
I'd expect 2 while
loops that are not nested. First to read the offset
bytes, next to read the bytes
.
Further this could be sub-function calls. Something like:
if (read_bytes(handle, offset, no_echo) == OK) {
if (read_bytes(handle, bytes, echo_to_stdout) == OK) {
success();
}
}
fail();