Well, it's obvious you are thinking along the proper lines of logic to get to the simulated tail
of lines, but you appear to be stumbling on how to approach handling the memory allocation, reallocation and freeing. (which is likely to point of the exercise).
While there is nothing that prevents you from allocating your pointers for pa
in main()
and passing that parameter to readlines()
, that is somewhat an awkward way to do it. When you think of creating a function that will allocated storage for an object, let the function allocate for the complete object and return a pointer to the object on success, or return NULL on failure. That way the calling function knows if the function returns a valid pointer it is responsible for freeing the memory associated with the object (instead of some of the memory being allocated in different places). If the function returns NULL
-- the caller knows the function failed and it need not worry about any memory for the object.
This also frees you from having to pass a parameter for the object. Since you will allocate for a complete object in the function, simply change the return type to the type of your object, (char**
here) and pass a pointer-to the memory holding the number of lines to output. Why a pointer? If less than that number of lines are stored (either because the file being read has fewer lines, or you ran out of memory before storing all lines), you can update the value at that address with the actual number of lines stored and make that number available back in the caller (main()
here).
With those changes, you can declare your function as:
char **readlines (int *n)
{
Within your function you need to declare a line-counter, a buffer to hold the line read from the file (which I presume your MAXLEN
is for) and declare and allocate the pointers for your object, validating every allocation. For example:
int ndx = 0; /* line counter */
char buf[MAXLEN], **pa = malloc (*n * sizeof *pa); /* allocate pointers */
if (!pa) { /* validate pointer allocation */
perror ("malloc-pa");
return pa;
}
for (int i = 0; i < *n; i++) /* initialize all pointers NULL */
pa[i] = NULL;
Note above, the pointers have all been initialized NULL
which will allow both the initial allocation and any needed reallocations to be handled by realloc()
. Also note that instead of using malloc
for the pointers, you can use calloc
which will set all bytes zero (and for all compilers I know, cause the pointers to evaluate as NULL
without having to explicitly loop setting them). However that isn't guaranteed by the standard -- so a loop is proper.
Here fgets()
is used to read each line and strcspn()
is used to trim the '\n'
and get the length of each line -- you can use whatever function you like. After the line is read, trimmed and length obtained, the memory is allocated (or reallocated) to hold the line and the line is copied to the new block of memory. Your nlines % n
index is thinking correctly, but you don't increment nlines
until after the allocation and assignment, e.g.
(note: Edited below to treat any line reallocation failure as terminal, and Free All Memory returning NULL
as discussed in the comments with @4386427 -- needed due to cyclical use of indexes and any failure after all lines originally allocated would result in unusable partial results (non-sequential line output))
while (fgets (buf, MAXLEN, stdin)) { /* read each line of input */
void *tmp; /* tmp to realloc with */
size_t len; /* line length */
buf[(len = strcspn (buf, "\n"))] = 0; /* trim '\n', get length */
/* always realloc to a temporary pointer, validate before assigning */
if (!(tmp = realloc (pa[ndx % *n], len + 1))) {
int rm = ndx > *n ? *n : ndx; /* detrmine no. of lines to free */
perror ("realloc-pa[ndx % *n]");
while (rm--) /* loop freeing each allocated line */
free (pa[rm]);
free (pa); /* free pointers */
return NULL;
}
pa[ndx % *n] = tmp; /* assign new block to pa[ndx%n] */
memcpy (pa[ndx % *n], buf, len + 1); /* copy line to block of memory */
ndx++; /* increment line count */
}
(note: if allocation fails for any allocated line, all allocated lines are freed along with the pointers and NULL
is returned avoiding any memory leak. Your continued overwriting of each pointer with a new address for each newly allocated block with continually leak memory that can no longer be freed -- you have lost the start address to the original block when you overwrite the pointer)
The final thing you do before returning your allocated object is to check if your index is less than the value for *n'
and if it is, update the value at that address so the actual number of lines stored is available back in the caller, e.g.
if (ndx < *n) /* if less than *n lines read */
*n = ndx; /* update number at that address with ndx */
return pa; /* return allocated object */
}
That's basically it for your function. Putting it altogether with the output simply written from main()
, you would have:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#define NLINES 10 /* default number of lines */
#define MAXLEN 1000 /* max characters per-line */
/* create and store last *n lines from stdin in allocated object,
* returning pointer to object on success, and updating value at n,
* if less than NLINES lines read. Return NULL on failure. Caller
* is responsible for freeing allocated memory.
*/
char **readlines (int *n)
{
int ndx = 0; /* line counter */
char buf[MAXLEN], **pa = malloc (*n * sizeof *pa); /* allocate pointers */
if (!pa) { /* validate pointer allocation */
perror ("malloc-pa");
return pa;
}
for (int i = 0; i < *n; i++) /* initialize all pointers NULL */
pa[i] = NULL;
while (fgets (buf, MAXLEN, stdin)) { /* read each line of input */
void *tmp; /* tmp to realloc with */
size_t len; /* line length */
buf[(len = strcspn (buf, "\n"))] = 0; /* trim '\n', get length */
/* always realloc to a temporary pointer, validate before assigning */
if (!(tmp = realloc (pa[ndx % *n], len + 1))) {
int rm = ndx > *n ? *n : ndx; /* detrmine no. of lines to free */
perror ("realloc-pa[ndx % *n]");
while (rm--) /* loop freeing each allocated line */
free (pa[rm]);
free (pa); /* free pointers */
return NULL;
}
pa[ndx % *n] = tmp; /* assign new block to pa[ndx%n] */
memcpy (pa[ndx % *n], buf, len + 1); /* copy line to block of memory */
ndx++; /* increment line count */
}
if (ndx < *n) /* if less than *n lines read */
*n = ndx; /* update number at that address with ndx */
return pa; /* return allocated object */
}
int main (int argc, char **argv) {
char *p = NULL, **lines = NULL; /* pointers for strtol, and lines */
int n = argc > 1 ? (int)strtol (argv[1], &p, 0) : NLINES;
if (n != NLINES && (errno || p == argv[1])) { /* validate conversion */
fprintf (stderr, "error: invalid no. of lines '%s'\n", argv[1]);
return 1;
}
if (!(lines = readlines(&n))) { /* read lines validate return */
fputs ("error: readlines failed.\n", stderr);
return 1;
}
for (int i = 0; i < n; i++) { /* loop over each stored line */
puts (lines[i]); /* output line */
free (lines[i]); /* free storage for line */
}
free (lines); /* free pointers */
}
(you can add the functions you like to replace the read with fgets()
and the output loop in main()
, as desired).
Example Use/Output
Default behavior:
$ printf "%s\n" line{1..20} | ./bin/tail
line11
line12
line13
line14
line15
line16
line17
line18
line19
line20
Output only 5
lines instead of default:
$ printf "%s\n" line{1..20} | ./bin/tail 5
line16
line17
line18
line19
line20
Handle less than default number of lines in file:
$ printf "%s\n" line{1..5} | ./bin/tail
line1
line2
line3
line4
line5
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind
is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ printf "%s\n" line{1..20} | valgrind ./bin/tail 5
==25642== Memcheck, a memory error detector
==25642== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==25642== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==25642== Command: ./bin/tail 5
==25642==
line16
line17
line18
line19
line20
==25642==
==25642== HEAP SUMMARY:
==25642== in use at exit: 0 bytes in 0 blocks
==25642== total heap usage: 23 allocs, 23 frees, 5,291 bytes allocated
==25642==
==25642== All heap blocks were freed -- no leaks are possible
==25642==
==25642== For counts of detected and suppressed errors, rerun with: -v
==25642== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Look things over and let me know if you have further questions.