-3

I am trying to concatenate two values that are returned from two different functions, a Server IP Address and a Student ID. The functions used to get these have been void function in order to access the server. I am wanting to use strcat() function but I can't get it to work successfully. The following code successfully returns both of the single values from the server.

void get_ipaddress(void)
{
   int fd;
   struct ifreq ifr;

   printf("IP Address:");

   fd = socket(AF_INET, SOCK_DGRAM, 0);

    /* I want to get an IPv4 IP address */
    ifr.ifr_addr.sa_family = AF_INET;

   /* I want an IP address attached to "eth0" */
    strncpy(ifr.ifr_name, "eth0", IFNAMSIZ-1);

   ioctl(fd, SIOCGIFADDR, &ifr);

   close(fd);

   /* Display result */
   printf("%s\n", inet_ntoa(((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr));
}


void get_studentid(int socket)
{
   char studentid[32];
   size_t k;

   readn(socket, (unsigned char *) &k, sizeof(size_t)); 
   readn(socket, (unsigned char *) studentid, k);

   printf("Student ID: %s\n", studentid);
} // end get_studentid()

I am trying to get the output

192.168.229.128: S1427722

Any code that I have previously tried returned the error

invalid use of void expression

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • 1
    where is strcat here? – Sudhee Dec 11 '17 at 01:29
  • it's not there, I have been doing trial and error on many different ways of using strcat() and each time I implement it, it returns "invalid use of expression", been at it for 3 hours so didn't know what implementation to put in – Jack McIvor Dec 11 '17 at 01:32
  • 1
    Oh - you are not returning any pointer both function. so you cannot use it inside strcat(). – Sudhee Dec 11 '17 at 01:33
  • thanks anyway, back to the drawing board – Jack McIvor Dec 11 '17 at 01:35
  • Note that you couldn't simply (legitimately) add `return student_id;` to `get_student()`, even if you changed the return type to `char *` because you'd be returning a pointer to a local variable, which is a No-No. You might need to use `strdup()` to create copies of the strings — `student_id` in `get_student()` and the result of `ntoa()` in `get_ipaddress()`, and with a change in return type, you could then return the copies of the strings. You'd have to make sure you freed the copies, too, or you'd leak memory. – Jonathan Leffler Dec 11 '17 at 01:41
  • 1
    "*The functions used to get these have been void function in order to access the server.*" What does this mean? – David Schwartz Dec 11 '17 at 02:01
  • @DavidSchwartz sorry, that was meant to be worded as "the functions used to get these have been through void functions" – Jack McIvor Dec 11 '17 at 02:51

1 Answers1

1

Your function get_ipaddress should either malloc and return an IPv4 address, or be given a string/char[] to fill. So you could change it to:

void get_ipaddress(char *addr, int n)
{
    /* ... */

    // Copy the string into our buffer
    strncpy(addr, inet_ntoa(((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr), n);
}

get_studentid should be the same:

void get_studentid(int socket, char *studentid, int n)
{
   size_t k;
   unsigned char *buf;

   readn(socket, (unsigned char *) &k, sizeof(size_t)); 
   buf = malloc(k);
   readn(socket, buf, k);

   strncpy(studentid, buf, n);
   free(buf);

} // end get_studentid()

And now in your main you can do:

int main(void) {
    char s_id[32] = {0};
    char ipaddr[32] = {0};
    char output[64] = {0};
    int sock;

    // Call the functions 
    get_ipaddress(ipaddr, 32);
    // Open your socket here
    get_studentid(sock, s_id, 32);

    // Format the output
    snprintf(output, 64, "%s: %s", ipaddr, s_id);
    // Print the result
    printf("%s\n", output);
}

This takes advantage of sprintf(), which is formatted printing to a string, for this purpose at least it is better than strcat, because for strcat you would need to concatenate twice (": " and studentid).

Jacob H
  • 864
  • 1
  • 10
  • 25
  • 3
    A function that takes a pointer to a buffer to fill in should **ALWAYS** also have an argument that is the size of the buffer. Otherwise you open up all kinds of buffer overflow problems. – Chris Dodd Dec 11 '17 at 02:38
  • @ChrisDodd You're right, I wrote the code quick but I should probably promote good practice. The answer has been updated. – Jacob H Dec 11 '17 at 02:48
  • @JacobH This is fantastic code but when it comes to implementing the code, I get a segmentation fault. When running the code from a menu option, should there being anything in () following the name of the function? – Jack McIvor Dec 11 '17 at 03:05
  • `strncpy` is dangerous because it does not properly null-terminate (too) long inputs, so you need to explicitly add a null terminator. Alternately, (and more flexibly) just use `snprintf` and return the return value of that (which is the (required) length), rather than returning void. – Chris Dodd Dec 11 '17 at 03:55