0

I was writing a code in which I created two processes, handler.c and calculate.c, which communicate using a named pipe. The handler.c recieves the operands and operator as command-line arguments and sends them to calculate.c, then reads the result and prints it. The calculate.c reads the operands and operator from the pipe, performs the calculation, and then writes the result to the pipe.

The problem is that when I read the result value from the pipe in handler.c, it always gives the same wrong value (822096384) no matter what I do. Below are the codes.

HANDLER.C

#include<stdio.h>
#include<unistd.h>
#include<stdlib.h>
#include<string.h>
#include<fcntl.h>

int main(int argc, char* argv[])
{
    char* name = argv[1];// pipe name
    int a, b;
    
    int fd = open(name, O_RDWR);
    
    a=atoi(argv[3]);
    b=atoi(argv[4]);

    write(fd, &a, sizeof(int));
    sleep(1);
    write(fd, &b, sizeof(int));
    sleep(1);
    write(fd, argv[2], sizeof(argv[2]));
    int res = 0;
    sleep(1);
    read(fd, &res, sizeof(int)); //it read the wrong value i.e. 822096384
    printf("%d", res);
    printf("\n %s %s %s = %d\n", argv[3], argv[2], argv[4], res);
    close(fd);

return 0;
}

CALCULATE.C

#include<stdio.h>
#include<unistd.h>
#include<stdlib.h>
#include<string.h>
#include<fcntl.h>

int main(int argc, char* argv[])
{
    char* name = argv[1];   
    int res = 0;
    int a,b;
    char o;
    int fd = open(name, O_RDWR);
    


    read(fd, &a, sizeof(int));
    printf("op1: %d ", a);
    sleep(1);
    read(fd, &b, sizeof(int));
    printf("op2: %d ", b);
    sleep(1);
    read(fd, &o, sizeof(char));
    printf("operator: %c ", o);

    if(strcmp(&o, "+") == 0)//if(o = '+')
    {
        res = a + b;
    }

    else if(strcmp(&o, "/") == 0)
    {
        res = a / b;
    }

    else if(strcmp(&o, "-") == 0)
    {
        res = a - b;    
    }

    else if(strcmp(&o, "*") == 0)
    {
        res = a * b;
    }
    printf("result: %d\n", res); //here res have the correct value
    write(fd, &res, sizeof(res));
    close(fd);
return 0;
}

Output of Hander.c

--bash
User@V-Ubuntu:~/Desktop/pipes$ ./handler pipe + 2 1
822096384
 2 + 1 = 822096384

Output of Calculate.c

--bash
User@V-Ubuntu:~/Desktop/pipes$ ./calculate pipe
op1: 2 op2: 1 operator: + result 3
John Bollinger
  • 160,171
  • 8
  • 81
  • 157
Qaisar Mateen
  • 23
  • 6
  • 2
    I bet it didn't read any value at all, and it told you it didn't read a value, and it told you why, but you ignored it. – user253751 Mar 09 '23 at 17:10
  • is it even valid to open a pipe with O_RDWR? – user253751 Mar 09 '23 at 17:10
  • 2
    You are not doing any error checking, beginning from blindly hoping the program arguments are valid. – Weather Vane Mar 09 '23 at 17:11
  • 2
    The code contains undefined behaviour with `if(strcmp(&o, "+")..` because `char o;` is not a C string. – Weather Vane Mar 09 '23 at 17:16
  • agree with what @WeatherVane said, if you're working directly with system calls like open, read, write, etc., you need to be checking the return values to make sure each call is working. in fact, you should always be checking return values for c library functions. – squidwardsface Mar 09 '23 at 17:29
  • 2
    It seems pipe isn't really a pipe but a file and you're just reading what you wrote. The value 822096384 is 0x31003200 which is "1" and "2". Those are likely coming from `argv[3]` and `argv[4]` because when you wrote the op, you said `sizeof(argv[2])` instead of `sizeof(argv[2][0])` writing 8 bytes instead of 1. They are flipped because you're on a little endian machine. If you invoke as + 1 2 instead the value will be 838,873,344 (0x32003100). At least that's my guess. – Tony Lee Mar 09 '23 at 17:31
  • ok thank you all for the help, I will now try to implement proper error check – Qaisar Mateen Mar 09 '23 at 17:35
  • 1
    @TonyLee i tried what yoy said about `Sizeof(argv[2][0])` and it worked for me, thanks – Qaisar Mateen Mar 09 '23 at 17:43
  • Side note: https://stackoverflow.com/questions/1658476/c-fopen-vs-open – xihtyM Mar 09 '23 at 18:15
  • 1
    It is problematic to use a single pipe for bidirectional communication. It's hard to ensure that each participant reads only the data written by the other, and not data that it wrote itself. I presume that all the `sleep()` calls in the code are an attempt to mitigate that, and they may succeed at that much of the time, but that's not reliable. If you're going to comminucate via pipes, whether named or anonymous, then it would be better to use a separate pipe for each direction. – John Bollinger Mar 09 '23 at 18:29
  • @JohnBollinger I am very much aware of this fact, but you know teachers.... – Qaisar Mateen Mar 11 '23 at 08:36
  • I have known many teachers in many fields, @QaisarMateen. Quite enough so to have no hesitation about opining that either you are missing something about the assignment, or else your particular teacher has chosen this assignment poorly. In the "missing something" department might come something like being able to use something else to mediate use of the pipe, such as a pair of semaphores, if you're not allowed to just use a second pipe. – John Bollinger Mar 11 '23 at 16:41
  • @TonyLee, I agree that it looks like the program is probably reading back what it wrote itself. However, that is consistent with the file in question being a named pipe. More so, in fact, than with it being a regular file, since if it were a regular file then you would expect the program to have to reposition to the beginning of the file to read back what it previously wrote. – John Bollinger Mar 11 '23 at 16:50

1 Answers1

0

I was unable to recreate your issue but you might want to add an error check like so:

handler.c

#include<stdio.h>
#include<unistd.h>
#include<stdlib.h>
#include<string.h>
#include<fcntl.h>

int main(int argc, char* argv[])
{
    char* name = argv[1];// pipe name
    int a, b;

    int fd = open(name, O_RDWR);

    a=atoi(argv[3]);
    b=atoi(argv[4]);

    write(fd, &a, sizeof(int));
    sleep(1);
    write(fd, &b, sizeof(int));
    sleep(1);
    write(fd, argv[2], sizeof(argv[2]));
    int res = 0;
    sleep(1);
    if (read(fd, &res, sizeof(int)))
    {
      printf("%d", res);
    }
    else 
    {
      printf("Could not read from buffer\n"); // Add an error check here
      close(fd);
      exit(EXIT_FAILURE);
    }
    
    printf("\n %s %s %s = %d\n", argv[3], argv[2], argv[4], res);
    close(fd);

return 0;
}

And for calculate.c - strcmp isn't necessary. C uses integer values for characters under the hood so you can go ahead and just check direct comparison like so:

#include<stdio.h>
#include<unistd.h>
#include<stdlib.h>
#include<string.h>
#include<fcntl.h>

int main(int argc, char* argv[])
{
    char* name = argv[1];   
    int res = 0;
    int a,b;
    char o;
    int fd = open(name, O_RDWR);
    


    read(fd, &a, sizeof(int));
    printf("op1: %d ", a);
    sleep(1);
    read(fd, &b, sizeof(int));
    printf("op2: %d ", b);
    sleep(1);
    read(fd, &o, sizeof(char));
    printf("operator: %c ", o);

    if(o == '+')
    {
        res = a + b;
    }

    else if(o == '/')
    {
        res = a / b;
    }

    else if(o == '-')
    {
        res = a - b;    
    }

    else if(o == '*')
    {
        res = a * b;
    }
    
    printf("result: %d\n", res); //here res have the wright value
    write(fd, &res, sizeof(res));
    close(fd);
return 0;
}

Why isn't it necessary? Well, when you pass the address of a char it does indeed get passed to the function without any warnings but there is no null byte to tell the function when to terminate the string thus inducing what would be known as undefined errors.

Ty Cooper
  • 95
  • 1
  • 5