Need some help with a program that will exec two different commands, with the output of first going into input of second, or with 3 or more, it chains the cmds.
relevant code in paste bin :)
#Broken pipe error (sigpipe)
sterile dew
half tuskBOT
sterile dew
All the important functions have pipe, exec, or smth along those lines in their name
would appreciate any help or advice
void python
may i suggest asan?
what do you suppose this c for (int i = 0; i < pm->pipeCount; i++) { int fp[PIPE_ENDS]; pipe(fp); pm->pipes[i] = fp; }
does exactly?
sterile dew
may i suggest asan?
what do you suppose this ```c
for (int i = 0; i < pm->pipeC...
what is asan?
sterile dew
may i suggest asan?
what do you suppose this ```c
for (int i = 0; i < pm->pipeC...
it makes an int array, turns it into a pipe (assigns two file descriptors), then adds that pipe to an array of pipes
is this wrong?
should i use malloc?
void python
what is asan?
Address Sanitizer
void python
is this wrong?
Is this valid:
int *a;
{
int tmp = 0;
a = &tmp;
}
(*a)++;```?ill be honest, i dont know
tbh, id say yes?
sterile dew
Is this valid:
```c
int *a;
{
int tmp = 0;
a = &tmp;
}
(*a)++;```?
also ive tried to redo the code
ill send what its at currently
tender knot
may i suggest asan?
what do you suppose this ```c
for (int i = 0; i < pm->pipeC...
As soon as the scope ends, variables declared on the stack in that scope are destroyed. Here, fp is declared inside the scope of an iteration of the for loop. As soon as the for loop goes on to the next iteration fp is destroyed and any pointer to it is therefore invalid
void python
https://pastebin.com/UkxmMX6J
Well that code is incorrect (logically) do you really want only 2 pipes?
And regardless of your answer - it doesn't seem you call pipe on them at all (i.e. you never initialize them)
you could use
int (*pipes)[PIPES_ENDS) = malloc(sizeof *pipes * pipes_amount);```
where `PIPES_ENDS` is `2`.
Otherwise you can manually `*alloc` each and every pipeAlternatively you can do something along these lines
https://github.com/nofunadler/simple-shell/blob/93b8dae6949870dde3dcb00c1352179a0910b5c3/lib/util/pipes.c#L15
(sorry for the delay, a bit busy these days)
void python
tbh, id say yes?
And ah, just in case it isn't clear - the answer is no, as @tender knot kindly explained
sterile dew
ah okay
sterile dew
@void python what actually is the best way for me to do what im trying to do
sterile dew
Well that code is incorrect (logically) do you really want only 2 pipes?
And re...
so my idea was
i only ever need at most 2 pipes
one to get the read end of the previous commands, and one to write my current command output to
and thats for when i have 3 commands
if i have 2 commands, only 1 pipe is need (number of commands - 1)
so if i just instantiate pipes as i need id be all fine and dandy
and i only need to make 2
void python
the robust solution would be either the first one i suggested or the last. you can choose between the 2
sterile dew
what was the first thing you suggested sorry?
the second was the malloc an array for size of 2 * number of pipes?
ohh right, in the link
oops ahaha
void python
if you go ahead and look at the link posted you'll find a similar program with a similar solution(s) [one i wrote sometime ago to help someone on some other server). that program isn't perfect - far from it - but you can use it as a reference should you want to.
sterile dew
so actually, that was what i tried
ill see if i can get it back, one second
PipeManager* pipe_manager(Params params) {
PipeManager* pm = malloc(sizeof(PipeManager));
pm->pipeCount = params.cm->numCmds - 1;
//pm->stdoutfp = dup(STDOUT_FILENO);
pm->pipes = malloc(sizeof(int[PIPE_ENDS]) * 2);
for (int i = 0; i < pm->pipeCount; i++) {
pipe(pm->pipes[i]);
}
return pm;
}
smth like this?
void python
what was the first thing you suggested sorry?
first int (*pipes)[PIPES_ENDS] = malloc(...);
second
int **pipes = malloc(...);
if(...) { ... }
for(...) {
pipes[i] = malloc(...);
the last one you can see in the link above
sterile dew
typedef struct {
int** pipes;
int pipeCount;
int stdoutfp;
} PipeManager;
that won't work. this is pmuch the same as the last solution i suggested. if you're to use it - take note of the way i iterate through the array there.
to give some intuition:
// will create a large continuous block (array) in memory.
pm->pipes = malloc(sizeof(int[PIPE_ENDS]) * 2);
for (int i = 0; i < pm->pipeCount; i++) { // will iterate over _each_ int
pipe(pm->pipes[i]); // pipes expects a block (or array) of the size of 2 ints
}```
meaning if you call `pipe` on `pipes[0]` - it'll init both `pipes[0]` _and_ `pipes[1]`. calling `pipe` on `pipes[1]` again - is a mistakeof the solutions, what do you think is the best approach?
void python
the first
sterile dew
so, the struct should be like this:
typedef struct {
int (*pipes)[PIPE_ENDS];
int pipeCount;
int stdoutfp;
} PipeManager;
not quite. int (*pipes)[PIPE_ENDS]. i.e. pipes is a pointer to an array for size 2
if you haven't used pointers to array before - consider picking one of the other 2 options
sterile dew
i have used, but only for chars*
like, char** pointers
in a previous thing i was keeping a dictionary of words and adding/removing from it as needed
void python
those aren't the same. meaning you haven't. please consider another solution
sterile dew
i mean, i think i understand how they work
when u initialise ur pointer to an array
the pointer points to the very first object
void python
no
sterile dew
oh, im rly misunderstanding what happens then
void python
when one init a ptr to an array - the ptr points to an array
sterile dew
when one init a ptr to an array - the ptr points to an array
but doesnt that point to the start of the array?
id like to learn/practise if its important
especially if its a common practise (which i assume it is)
void python
it points to the array itself. not the first object of said array
void python
id like to learn/practise if its important
consider this then:
void foo(int **mat, size_t rows, size_t cols);
void bar(int mat[ROWS][COLS], size_t rows, size_t cols);
void baz(int (*mat)[COLS], size_t rows, size_t cols);
void func(size_t rows, size_t cols, int mat[rows][cols]);
int main(void) {
int mat[1][1] = {0};
foo(mat, 1, 1); // incorrect
bar(mat, 1, 1); // correct but is a 'lie'
baz(mat, 1, 1); // correct and true
func(1, 1, mat); // correct but VLA
}```VLA?
void python
variable length array (which in the above example is one of the 'good' uses of it)
sterile dew
and what do you mean by lie?
void python
int mat[ROWS][COLS] as a function parameter hides the true type of mat. since mat is a ptr to a ptr when one pass it to a function, a pointer decay happens turning int[1][1] into int (*)[1] (i.e. a pointer to an array for size one
void python
because of it being a VLA. meaning its type is that VLA and ofc we cannot talk about VLAs without https://stackoverflow.com/a/54163435 (which i really recommend reading when you got the time)
sterile dew
also, to get the 1,1 element, id use *((mat + 1) + 1)
sterile dew
because of it being a VLA. meaning its _type_ is that VLA and ofc we cannot talk...
okay ill bookmark this
void python
also, to get the 1,1 element, id use `*((mat + 1) + 1)`
in the above example that would be illegal. but no.
mat[i][j] for bar baz or func
as mentioned foo is incorrect
sterile dew
in the above example that would be illegal. but no.
`mat[i][j]` for `bar` `baz` ...
ah okay, so to access the memory i can just use mat[i][j] for a pointer to an array
that was silly of me actually
so my struct:
typedef struct {
int (*pipes)[PIPE_ENDS];
int pipeCount;
int stdoutfp;
} PipeManager;
then if ive got a pointer to my struct i can do:
pm->pipes = malloc((sizeof(*pipes) * pipeCount);
one can always try 🙂
sterile dew
then if ive got a pointer to my struct i can do:
```c
pm->pipes = malloc((sizeo...
thos this is okay then?
void python
still no
sterile dew
wait what?
void python
the parenthasis change the meaning of the expression in this case
sterile dew
oh really??
dont you need parentheses for sizeof?
oh wait, this
i bracketed wrong i think
pm->pipes = malloc((sizeof(*pipes) * pipeCount);
i want pipecount number of *pipes sized memory
void python
yep
sizeof(*pipes * pipeCount) is just UB
sizeof *pipes * ... means take the size of whatever is the type of *pipes (i.e. an array of size of 2) and multiply it by some amount
sterile dew
i just forgot to change the bracketing from the previous thing i typed ahha
so now
do i want to open all my pipes at once?
void python
dont you need parentheses for sizeof?
no. sizeof isn't a function. its an operator. you can use it with / out parenthasis depends on your operands
sterile dew
no. `sizeof` isn't a function. its an operator. you can use it with / out parent...
oh i didnt know that haha
void python
well now you do
sterile dew
thank you!
sterile dew
do i want to open all my pipes at once?
i ask this, because i was thinking my program may hang if pipes are open
void python
```c
pm->pipes = malloc((sizeof(*pipes) * pipeCount);
```
this is fine aside from the redundant opening parent (though personally i don't like putting parenthesis where there's no need for them)
void python
i ask this, because i was thinking my program may hang if pipes are open
up to you. the state of the pipes shouldn't affect your program (assuming your program doesn't try to read / write from said pipes)
sterile dew
up to you. the state of the pipes shouldn't affect your program (assuming your p...
oh really? i thought that any open pipes at all would cause hanging
okay so i wont worry about closing them then
void python
oh really? i thought that any open pipes at all would cause hanging
i can't control your thoughts 🙃
why would it cause something to hand if said something doesn't interact with them?
sterile dew
tbh i thought it was a potential reason, i didnt think this was the case till my program started hanging
but that was probably because of other reasons
void python
okay so i wont worry about closing them then
you actually should in this case. otherwise it way cause problems later down the line (and its best practice anyway)
sterile dew
you actually should in this case. otherwise it way cause problems later down the...
ah okay
pm->pipes = malloc(sizeof(*pipes) * pm->pipeCount);
am i writing this wrong?
void python
yes. count your parenthesis
void python
this is fine aside from the redundant opening parent (though personally i don't ...
also 
sterile dew
malloc doesnt need parentheses?
void python
it most certainly does
sterile dew
uqfindexec.c:249:33: error: ‘pipes’ undeclared (first use in this function); did you mean ‘pipe’?
pm->pipes = malloc(sizeof (*pipes) * pm->pipeCount);
^~~~~
pipe
uqfindexec.c:249:33: note: each undeclared identifier is reported only once for each function it appears in
how many ( do you have as oppose to ) in that expression?
void python
```c
uqfindexec.c:249:33: error: ‘pipes’ undeclared (first use in this function)...
right. that too
sterile dew
how many `(` do you have as oppose to `)` in that expression?
2 opening, 2 closing?
am i blind?
void python
```c
pm->pipes = malloc(sizeof(*pipes) * pm->pipeCount);
```
i was referring to
sterile dew
right. that too
but im not sure why this is undeclared
void python
all good. as for the error is there an identifier (a var) named pipes anywhere?
sterile dew
oh i see what i need to do
pm->pipes = malloc (sizeof *pm->pipes * pm->pipeCount);
that compiles
i realise it doesnt know what pipes comes from
void python
that compiles
yep
sterile dew
then, would opening all my pipes now be logical?
void python
again - up to you. but yes - you can init them all
sterile dew
PipeManager* pipe_manager(Params params) {
PipeManager* pm = malloc(sizeof(PipeManager));
pm->pipeCount = params.cm->numCmds - 1;
//pm->stdoutfp = dup(STDOUT_FILENO);
pm->pipes = malloc(sizeof *pm->pipes * pm->pipeCount);
for (int i = 0; i < pm->pipeCount; i++) {
pipe(pm->pipes[i]);
}
return pm;
}
i believe this would work since it indexes each int[2]
void python
yep. looks like you got the idea
sterile dew
great!!
now i guess i can close and open as i need
also, each fork is completely memory independent right?
unless i have shared memory
i think
void python
yes to both (beside shared memory blocks which aren't present in your case)
void python
depends on the hour - but i promise to check in every once in a while
sterile dew
okay ty!
void python
np
sterile dew
void cmd_pipeliner(Params params, int cmd) {
if (params.cm->numCmds == 1) {
return;
}
if (cmd == 0) {
close(params.pm->pipes[cmd][READ_END]);
dup2(params.pm->pipes[cmd][WRITE_END], STDOUT_FILENO);
close(params.pm->pipes[cmd][WRITE_END]);
}
else if ((cmd + 1) == params.cm->numCmds) {
close(params.pm->pipes[cmd - 1][WRITE_END]);
dup2(params.pm->pipes[cmd - 1][READ_END], STDIN_FILENO);
close(params.pm->pipes[cmd - 1][READ_END]);
} else {
dup2(params.pm->pipes[cmd - 1][READ_END], STDIN_FILENO);
close(params.pm->pipes[cmd - 1][READ_END]);
dup2(params.pm->pipes[cmd][WRITE_END], STDOUT_FILENO);
close(params.pm->pipes[cmd][WRITE_END]);
}
for (int i = 0; i < params.pm->pipeCount; i++) {
//close(params.pm->pipes[i][READ_END]);
//close(params.pm->pipes[i][WRITE_END]);
}
}
okay i THINK, this is right?
if i have an array of cmds, and im passing in the index of the cmd
e.g. {echo, tr A-Z a-z, wc} or smth
id have 2 pipes
fork0 redirects its STDOUT to writing end of pipe0, fork1 redirects reading end of pipe1 to STDIN, and its STDOUT to write end of pipe1, then fork2 will redirect pipe1 read end to STDIN
and voila magic?
void python
i'm a bit confused by this block if i'm honest
else if ((cmd + 1) == params.cm->numCmds) { // why +1
close(params.pm->pipes[cmd - 1][WRITE_END]); // why -1
dup2(params.pm->pipes[cmd - 1][READ_END], STDIN_FILENO); // why -1
close(params.pm->pipes[cmd - 1][READ_END]); // why -1
} else {
```Params prepare_command(Params params)
{
DirectoryManager dm = directory_manager_initialiser(params);
params.stats = stats_initialiser();
params.pm = pipe_manager(params);
int childrenCount = 1;
pid_t* children = malloc(sizeof(pid_t) * childrenCount);
for (int i = 0; i < dm.entryCount; i++) {
int wstatus;
AbsPath path
= sbuf_initialiser(params.directory, dm.entries[i]->d_name);
for (int j = 0; j < params.cm->numCmds; j++) {
pid_t child;
if (!S_ISDIR(path.sbuf.st_mode) && !(child = fork())) {
params = every_file_handler(params, path);
execute_commands(params, path.absolutePath, path.path, j);
}
if ((j + 1) < params.cm->numCmds) {
close(params.pm->pipes[j][WRITE_END]);
}
children[childrenCount - 1] = child;
childrenCount++;
wait(NULL);
waitpid(child, &wstatus, 0);
update_statistics(params.stats, wstatus);
}
}
free_directory(dm);
return params;
}
if the cmd is the final one
then u want to redirect its input from the pipe, but you dont want to redirect its output
void python
if the cmd is the final one
so... why the -1?
sterile dew
so... why the -1?
there are always less pipes than cmds i think
so for [echo, wc]
pipes = [pipe0]
index of wc = 1, so we want pipes[0]
i think thats okay?
void python
aight. fair enough. that logic seem to be correct
sterile dew
so, i tried running just then and my program was hanging
so im not sure where
this is what happens in terminal
void python
did your parent close all pipes it didn't need?
sterile dew
uhh, i think so
1 second
ill send what my parent looks like
Params prepare_command(Params params)
{
DirectoryManager dm = directory_manager_initialiser(params);
params.stats = stats_initialiser();
params.pm = pipe_manager(params);
int childrenCount = 1;
pid_t* children = malloc(sizeof(pid_t) * childrenCount);
for (int i = 0; i < dm.entryCount; i++) {
int wstatus;
AbsPath path
= sbuf_initialiser(params.directory, dm.entries[i]->d_name);
for (int j = 0; j < params.cm->numCmds; j++) {
pid_t child;
if (!S_ISDIR(path.sbuf.st_mode) && !(child = fork())) {
params = every_file_handler(params, path);
execute_commands(params, path.absolutePath, path.path, j);
}
if ((j + 1) < params.cm->numCmds) {
close(params.pm->pipes[j][WRITE_END]);
}
children[childrenCount - 1] = child;
childrenCount++;
waitpid(child, &wstatus, 0);
update_statistics(params.stats, wstatus);
}
}
free_directory(dm);
return params;
}
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "echo {} | wc"
1 1 9
uqfindexec
0 0 0
uqfindexec.c
0 0 0
uqfindexec.o
Segmentation fault
this is my current output
oh no, i fixed it
i forgot to change my close condition
void python
as a side note
wait(NULL);
waitpid(child, &wstatus, 0);
isn't what you want and just bad.
either install a signal handler, or do something along the line of https://github.com/nofunadler/simple-shell/blob/93b8dae6949870dde3dcb00c1352179a0910b5c3/main.c#L88
regardless of your decision - your parent shouldn't wait on the child there. it should instead launch all children without waiting, and only start waiting after the last one has been launched
sterile dew
as a side note
```c
wait(NULL);
waitpid(child, &wstatus,...
oh i see
so actually, yea this is one of the things we need to implement, parallel vs linear running of the cmds
the wait(NULL) was smth i forgot to delete from my testing btw
void python
so actually, yea this is one of the things we need to implement, parallel vs lin...
i suggest using the above repo as a reference then
sterile dew
so, if i keep my array of pids
i can wait for all the pids if i put it outside of the j loop right?
actually, id have a separate loop to reap the children
void python
since i'm not sure about the purpose of your outer loop - it may or may not be correct 🙂
sterile dew
since i'm not sure about the purpose of your outer loop - it may or may not be c...
ah yep fair enough
although i realise im actually missing some input
s4773517@moss:~/repo/trunk/a3$ ls
Makefile testfiles uqfindexec uqfindexec.c uqfindexec.o
but
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "echo {} | wc"
1 1 9
uqfindexec
0 0 0
uqfindexec.c
0 0 0
uqfindexec.o
and this shouldnt even be printing the program names
s4773517@moss:~/repo/trunk/a3$ demo-uqfindexec "echo {} | wc"
1 1 9
1 1 11
1 1 13
1 1 13
this is the expected output
do i need to flush my buffer?
fflush(stdout)?
sterile dew
```
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "echo {} | wc"
1 1 ...
this behaviour means my pipe is actually not working as intended right?
void python
probably, though i'm not sure what echo {} suppose to do other than printing {} to stdout
sterile dew
so echo {} just runs echo on all the files in the directory
ive just made it so my program will run a cmd on every file in a directory if {} is supplied
it just replaces {} with the file name
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "ls | wc"
5 5 56
Makefile testfiles uqfindexec uqfindexec.c uqfindexec.o
0 0 0
Makefile testfiles uqfindexec uqfindexec.c uqfindexec.o
0 0 0
Makefile testfiles uqfindexec uqfindexec.c uqfindexec.o
0 0 0
s4773517@moss:~/repo/trunk/a3$ demo-uqfindexec "ls | wc"
5 5 56
5 5 56
5 5 56
5 5 56
my output vs what should happen
void python
so echo {} just runs echo on all the files in the directory
is it?
sterile dew
```
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "ls | wc"
5 5 5...
so have i not piped properly?
sterile dew
is it?
oh not in normal linux ahhaha
just in my program
its a special input u can give it
the {}
void python
ah. i see. check you implementation of it first then i suppose
sterile dew
so my {} is definitely implemented correctly (without piping i mean)
like im super sure of it
void python
couldn't hurt to check
sterile dew
the fact that the file names are even appearing in the terminal suggests that im doing something weird
(to me at least)
like, no stdout from the first command should reach terminal right?
if ive piped properly?
void python
oh - i see your ls has the same problem. yeah that might be a piping issue
sterile dew
oh - i see your `ls` has the same problem. yeah that might be a piping issue
yea im a little confused
void python
well if ls is picking up things from stdin which shouldn't be there - it means someone put them there
sterile dew
well if `ls` is picking up things from `stdin` which shouldn't be there - it mea...
was my piping and closing correct?
void python
did your parent close all pipes it didn't need?
on that note - i forgot. if you create all your pipes before launching your processes - it means that each and every process have those mapping of said pipes. you'd need to close them in each and every child (after you configured them correctly ofc)
sterile dew
on that note - i forgot. if you create all your pipes _before_ launching your pr...
ohhh so i do need to do that
do i close the reading and writing end of all pipes?
or do i leave the reading end open for the next fork?
void python
i really do suggest you use that repo i linked as a reference
sterile dew
ah okay
void python
start in main and skim through it
void python
can't remember it was a long time ago. couple days maybe? it's not a well written one
sterile dew
how long after u started programming in c did u try it?
void python
well it was uploaded 8 months ago so 6 years ish
closer to 5
seems like it took one friday. again - its not a well written one. its good enough for a reference though
sterile dew
okay ill keep readng through
half tuskBOT
@sterile dew Has your question been resolved? If so, type !solved :)
sterile dew
okay so i seem to have fixed one of the issues
s4773517@moss:~/repo/trunk/a3$ ./uqfindexec "echo {} | wc"
1 1 9
uqfindexec
0 0 0
uqfindexec.c
0 0 0
uqfindexec.o
0 0 0
im actually getting my final output
sterile dew
@void python okay so i know what the issue is, my dup2 is actually failing
after the first command
im not sure why/how to fix, but i have identified dup2 fails
im pretty dumb for not recognising that wc is returning 0 for all the files
other than first
okay so im getting a bad file descriptor
when i print, both of the forks have the same file descriptor
OH OKAY
i needed to reinitialise my pipes for every file i go over
hell yes
its half working!
void python
glad you managed to figure it out.
minor comment though - when an API (or functions) returns something to indicate an error - one should check said value.
if one 1000% (yes one thousand percent) sure the value will never indicate an error one should cast the return value to void. by doing it one indicates to all readers of said code that:
- they are aware said function returns something
- yet they choose to ignore it willingly
in case of misbehaving programs - those casts would be pretty high on the 'things to check' list
sterile dew
glad you managed to figure it out.
minor comment though - when an API (or functi...
ahh right
i was pretty confident it shouldnt ever error (assuming i didnt write bad code lmao)
but iill do the void thing