diff mbox

Modify net/socket.c to use functions from include/qemu/sockets.h

Message ID 1462282457-24574-1-git-send-email-ashijeetacharya@gmail.com
State New
Headers show

Commit Message

Ashijeet Acharya May 3, 2016, 1:34 p.m. UTC 
From: Ashijeet <ashijeetacharya@gmail.com>

Replaced connect()/listen()/parse_host_port() in net/socket.c with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
 net/socket.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Comments

Daniel P. Berrangé May 3, 2016, 2:01 p.m. UTC | #1 
On Tue, May 03, 2016 at 07:04:17PM +0530, Ashijeet Acharya wrote:
> From: Ashijeet <ashijeetacharya@gmail.com>
> 
> Replaced connect()/listen()/parse_host_port() in net/socket.c
> with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.

If you're going to re-work this, then I think it'd be better to
go straight to using socket_connect() / socket_listen() / socket_parse()
which are QAPI based. The inet_* function should eventually be going
away once everything is using the QAPI based socket_* functions, so
adding more usage of them is not too desirable.


Regards,
Daniel
Ashijeet Acharya May 3, 2016, 6:13 p.m. UTC | #2 
On Tuesday 03 May 2016 07:31 PM, Daniel P. Berrange wrote:
> On Tue, May 03, 2016 at 07:04:17PM +0530, Ashijeet Acharya wrote:
>> From: Ashijeet <ashijeetacharya@gmail.com>
>>
>> Replaced connect()/listen()/parse_host_port() in net/socket.c
>> with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.
>
> If you're going to re-work this, then I think it'd be better to
> go straight to using socket_connect() / socket_listen() / socket_parse()
> which are QAPI based. The inet_* function should eventually be going
> away once everything is using the QAPI based socket_* functions, so
> adding more usage of them is not too desirable.
>

Yeah sure I can do that. Also I was wondering if there is a similar 
function for bind because the task listed on wiki BiteSized also expects 
replacement of bind(). So maybe adding/defining socket_bind() in 
util/qemu-sockets.c and the header include/qemu/sockets.h be helpful.

Thanks!
Ashijeet

> Regards,
> Daniel
>
Daniel P. Berrangé May 4, 2016, 8:39 a.m. UTC | #3 
On Tue, May 03, 2016 at 11:43:58PM +0530, Ashi wrote:
> 
> 
> On Tuesday 03 May 2016 07:31 PM, Daniel P. Berrange wrote:
> >On Tue, May 03, 2016 at 07:04:17PM +0530, Ashijeet Acharya wrote:
> >>From: Ashijeet <ashijeetacharya@gmail.com>
> >>
> >>Replaced connect()/listen()/parse_host_port() in net/socket.c
> >>with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.
> >
> >If you're going to re-work this, then I think it'd be better to
> >go straight to using socket_connect() / socket_listen() / socket_parse()
> >which are QAPI based. The inet_* function should eventually be going
> >away once everything is using the QAPI based socket_* functions, so
> >adding more usage of them is not too desirable.
> >
> 
> Yeah sure I can do that. Also I was wondering if there is a similar function
> for bind because the task listed on wiki BiteSized also expects replacement
> of bind(). So maybe adding/defining socket_bind() in util/qemu-sockets.c and
> the header include/qemu/sockets.h be helpful.

bind() is done automatically as part of the socket_listen() function

Regards,
Daniel
Ashijeet Acharya May 12, 2016, 9:09 a.m. UTC | #4 
On Wednesday 04 May 2016 02:09 PM, Daniel P. Berrange wrote:
> On Tue, May 03, 2016 at 11:43:58PM +0530, Ashi wrote:
>>
>>
>> On Tuesday 03 May 2016 07:31 PM, Daniel P. Berrange wrote:
>>> On Tue, May 03, 2016 at 07:04:17PM +0530, Ashijeet Acharya wrote:
>>>> From: Ashijeet <ashijeetacharya@gmail.com>
>>>>
>>>> Replaced connect()/listen()/parse_host_port() in net/socket.c
>>>> with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.
>>>
>>> If you're going to re-work this, then I think it'd be better to
>>> go straight to using socket_connect() / socket_listen() / socket_parse()
>>> which are QAPI based. The inet_* function should eventually be going
>>> away once everything is using the QAPI based socket_* functions, so
>>> adding more usage of them is not too desirable.
>>>
>>
>> Yeah sure I can do that. Also I was wondering if there is a similar function
>> for bind because the task listed on wiki BiteSized also expects replacement
>> of bind(). So maybe adding/defining socket_bind() in util/qemu-sockets.c and
>> the header include/qemu/sockets.h be helpful.
>
> bind() is done automatically as part of the socket_listen() function

Well, I have completed replacing all the older functions with new QAPI 
based socket_*() ones. Although I am stuck at a part which uses 
snprintf() in net_socket_connect_init() function (line no. 605). The 
inet_ntoa() and ntohs() used in snprintf() are really bugging me as I am 
not sure how to use them with the new SocketAddress type sockets. The 
code compiles without snprintf() but I am not sure what effect does it 
have as a whole to abort snprintf(). I am not sure what to do with that??

Thanks!
Ashijeet
>
> Regards,
> Daniel
>
Daniel P. Berrangé May 12, 2016, 9:18 a.m. UTC | #5 
On Thu, May 12, 2016 at 02:39:50PM +0530, Ashi wrote:
> On Wednesday 04 May 2016 02:09 PM, Daniel P. Berrange wrote:
> > On Tue, May 03, 2016 at 11:43:58PM +0530, Ashi wrote:
> > > 
> > > 
> > > On Tuesday 03 May 2016 07:31 PM, Daniel P. Berrange wrote:
> > > > On Tue, May 03, 2016 at 07:04:17PM +0530, Ashijeet Acharya wrote:
> > > > > From: Ashijeet <ashijeetacharya@gmail.com>
> > > > > 
> > > > > Replaced connect()/listen()/parse_host_port() in net/socket.c
> > > > > with inet_connect()/inet_listen/inet_parse() in include/qemu/sockets.h.
> > > > 
> > > > If you're going to re-work this, then I think it'd be better to
> > > > go straight to using socket_connect() / socket_listen() / socket_parse()
> > > > which are QAPI based. The inet_* function should eventually be going
> > > > away once everything is using the QAPI based socket_* functions, so
> > > > adding more usage of them is not too desirable.
> > > > 
> > > 
> > > Yeah sure I can do that. Also I was wondering if there is a similar function
> > > for bind because the task listed on wiki BiteSized also expects replacement
> > > of bind(). So maybe adding/defining socket_bind() in util/qemu-sockets.c and
> > > the header include/qemu/sockets.h be helpful.
> > 
> > bind() is done automatically as part of the socket_listen() function
> 
> Well, I have completed replacing all the older functions with new QAPI based
> socket_*() ones. Although I am stuck at a part which uses snprintf() in
> net_socket_connect_init() function (line no. 605). The inet_ntoa() and
> ntohs() used in snprintf() are really bugging me as I am not sure how to use
> them with the new SocketAddress type sockets. The code compiles without
> snprintf() but I am not sure what effect does it have as a whole to abort
> snprintf(). I am not sure what to do with that??

It is hard to answer that without you showing what your code conversion
currently looks like...

Regards,
Daniel
diff mbox

Patch

diff --git a/net/socket.c b/net/socket.c
index 9fa2cd8..20cc1d2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -524,8 +524,9 @@  static int net_socket_listen_init(NetClientState *peer,
     NetSocketState *s;
     struct sockaddr_in saddr;
     int fd, ret;
+    Error *local_error = NULL;
 
-    if (parse_host_port(&saddr, host_str) < 0)
+    if (inet_parse(host_str, &local_error) < 0)
         return -1;
 
     fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
@@ -543,7 +544,7 @@  static int net_socket_listen_init(NetClientState *peer,
         closesocket(fd);
         return -1;
     }
-    ret = listen(fd, 0);
+    ret = inet_listen(host_str, NULL, 256, SOCK_STREAM, 0, &local_error);
     if (ret < 0) {
         perror("listen");
         closesocket(fd);
@@ -568,8 +569,9 @@  static int net_socket_connect_init(NetClientState *peer,
     NetSocketState *s;
     int fd, connected, ret;
     struct sockaddr_in saddr;
+    Error *local_error = NULL;
 
-    if (parse_host_port(&saddr, host_str) < 0)
+    if (inet_parse(host_str, &local_error) < 0)
         return -1;
 
     fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
@@ -581,7 +583,7 @@  static int net_socket_connect_init(NetClientState *peer,
 
     connected = 0;
     for(;;) {
-        ret = connect(fd, (struct sockaddr *)&saddr, sizeof(saddr));
+        ret = inet_connect(host_str, &local_error);
         if (ret < 0) {
             if (errno == EINTR || errno == EWOULDBLOCK) {
                 /* continue */
@@ -618,8 +620,8 @@  static int net_socket_mcast_init(NetClientState *peer,
     int fd;
     struct sockaddr_in saddr;
     struct in_addr localaddr, *param_localaddr;
-
-    if (parse_host_port(&saddr, host_str) < 0)
+    Error *local_error = NULL;
+    if (inet_parse(host_str, &local_error) < 0)
         return -1;
 
     if (localaddr_str != NULL) {
@@ -656,12 +658,13 @@  static int net_socket_udp_init(NetClientState *peer,
     NetSocketState *s;
     int fd, ret;
     struct sockaddr_in laddr, raddr;
+    Error *local_error = NULL;
 
-    if (parse_host_port(&laddr, lhost) < 0) {
+    if (inet_parse(lhost, &local_error) < 0) {
         return -1;
     }
 
-    if (parse_host_port(&raddr, rhost) < 0) {
+    if (inet_parse(rhost, &local_error) < 0) {
         return -1;
     }