Reputation: 1
I am trying to read the entire data written from the user in a buffer to process it inside my own shell "called hsh" but the process is terminated with signal 13: Here is command that I passed to my shell:
valgrind echo " /bin/ls " | ./hsh
And here are the results:
==203856== Process terminating with default action of signal 13 (SIGPIPE)
==203856== at 0x4960077: write (write.c:26)
==203856== by 0x48E0E8C: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1181)
==203856== by 0x48E17A7: new_do_write (fileops.c:449)
==203856== by 0x48E17A7: _IO_new_file_xsputn (fileops.c:1255)
==203856== by 0x48E17A7: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1197)
==203856== by 0x48E037F: fputs_unlocked (iofputs_u.c:37)
==203856== by 0x10A7AE: ??? (in /usr/bin/echo)
==203856== by 0x4876082: (below main) (libc-start.c:308)
==203856==
==203856== HEAP SUMMARY:
==203856== in use at exit: 0 bytes in 0 blocks
==203856== total heap usage: 228 allocs, 228 frees, 25,267 bytes allocated
==203856==
==203856== All heap blocks were freed -- no leaks are possible
==203856==
==203856== For lists of detected and suppressed errors, rerun with: -s
==203856== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
here are my steps to read the data:
This is my code: Note: Every function that has underscore(_) in its name is implemented elsewhere and its prototype is declared in the "main.h" header file.
#include "main.h"
#define CHUNK_SIZE 1000
/**
* _getlines - Read the command lines
* @commands: The list where commands will be stored
* Return: On success - number of commands
* On error - "-1"
*/
int _getlines(char **commands)
{
char *line, *page, *chunk, ch, qoute;
ssize_t bytes_read, total_bytes_read = 0;
int j = 0, cmd_count = 0, i = 0, file_size;
line = malloc(CHUNK_SIZE * sizeof(char));
file_size = lseek(STDIN_FILENO, 0, SEEK_END);
lseek(STDIN_FILENO, 0, SEEK_SET);
page = malloc(file_size);
if (page == NULL)
return (-1);
while ((bytes_read = read(STDIN_FILENO, line, CHUNK_SIZE) > 0))
{
_strcpy(page + total_bytes_read, line);
total_bytes_read += bytes_read;
}
if (bytes_read == -1)
{
free(page);
return (-1);
}
printf("%s\n", page);
while (i < total_bytes_read)
{
ch = page[i++];
if (ch == '\"' || ch == '\'')
{
line[j++] = ch;
qoute = ch;
do {
ch = page[i++];
line[j++] = ch;
} while (ch != qoute);
}
else if (ch == '\n')
{
line[j++] = '\0';
commands[cmd_count] = _strdup(line);
j = 0;
(cmd_count)++;
}
else
line[j++] = ch;
}
if (cmd_count == 0)
{
commands[cmd_count] = _strdup(line);
cmd_count++;
}
free(line);
return (cmd_count);
}
Upvotes: 0
Views: 487
Reputation: 8344
While mostly neat and tidy code, there are many issues to be addressed. I've chopped away some bits to focus on lines with issues. (btw. Sending over 6000 SP's into your code will certainly cause line
to overflow as characters are copied to its 1000 byte length.)
int _getlines(char **commands) // Unknown size of ptr array being populated
char *line, *page, *chunk, ch, qoute; // Illegible block of declarations. "chunk" is unused.
ssize_t bytes_read, total_bytes_read = 0;
int j = 0, cmd_count = 0, i = 0, file_size; // "sizes" should be unsigned
line = malloc(CHUNK_SIZE * sizeof(char)); // sizeof a char is 1. Don't multiply
// Missing verification of success
file_size = lseek(STDIN_FILENO, 0, SEEK_END); // Not needed
page = malloc(file_size); // Did you mean "file_size + 1"?
if (page == NULL)
return (-1); // What about the allocation of "line"?
_strcpy(page + total_bytes_read, line); // "line" is not a string!
if (bytes_read == -1) {
free(page); // What about the allocation of "line"?
while (i < total_bytes_read) { // have to scroll back to verify i starts at 0
if (ch == '\"' || ch == '\'') {
line[j++] = ch; // No verification the j < size of line[]
line[j++] = ch; // again, here
} while (ch != qoute);
else if (ch == '\n') {
commands[cmd_count] = _strdup(line); // unnecessary allocation (see below)
line[j++] = ch; // and again, here
}
if (cmd_count == 0) // Not sure about this...
{
commands[cmd_count] = _strdup(line); // line is NOT necessarily a C string
cmd_count++;
}
free(line); // finally... but what about "page"?
return (cmd_count); // return is not a function call. () unnecessary
There are a few too many troubles to "lightly touch" to correct.
The following corrects these problems with a rather major overhaul. This code has, at least, a few comments explaining its operation.
int _getlines(char **commands, unsigned sz) { // array max size available
char k1024[ 1024 ], *page = NULL; // buffers here
ssize_t total = 0;
int i;
while( ( i = read( STDIN_FILENO, k1024, sizeof k1024 ) > 0 ) ) {
void *tmp = realloc( page, total + i + 1 ); // ongoing growth of input buffer
if( tmp == NULL ) {
// perhaps a diagnostic emitted here?
free( page );
return -1;
}
page = tmp;
memcpy( page + total, k1024, i );
total += i;
}
page[ total++ ] = 0; // terminate to form C string
i = 0;
while( isspace( page[ i ] ) ) i++; // toss away leading whitespace here
if( !page[ i ] ) {
free( page ); // that was a waste of time
return 0;
}
if( i )
memmove( page, page + i, total - i ); // shift everything to start of buffer
int cmd_count = 0;
commands[ cmd_count++ ] = page; // TODO: verify cmd_count doesn't exceed allowance
char qch = 0;
i = 0;
while( page[i] )
switch( page[i] ) {
case '\"':
case '\'':
if( !qch )
qch = page[ i ];
else if( page[ i ] == qch )
qch = 0;
i++;
break;
case '\n':
if( qch ) { i++; break; }
page[ i++ ] = 0;
if( page[ i ] )
commands[ cmd_count++ ] = page + i; // TODO: verify cmd_count
break;
default:
i++;
break;
}
return cmd_count;
}
The caller need only free( commands[0] )
to extinguish the entire block allocated. (Left to the reader to add the relevant headers.)
EDIT
Or... Perhaps the array of pointers to segments is meant to be dynamically allocated, too! Hard to say as the code calling this function has not been provided.
Upvotes: 0