| Message ID | 20240718164842.3650702-5-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/libxs: Fix SIGPIPE handling issues | expand |
On 18.07.24 18:48, Andrew Cooper wrote: > It will determine whether to use writev() or sendmsg(). > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Anthony PERARD <anthony.perard@vates.tech> > CC: Juergen Gross <jgross@suse.com> > --- > tools/libs/store/xs.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c > index 96ea2b192924..f4edeb05f03b 100644 > --- a/tools/libs/store/xs.c > +++ b/tools/libs/store/xs.c > @@ -65,6 +65,9 @@ struct xs_stored_msg { > struct xs_handle { > /* Communications channel to xenstore daemon. */ > int fd; > + > + bool is_socket; /* is @fd a file or socket? */ > + > Xentoolcore__Active_Handle tc_ah; /* for restrict */ > > /* > @@ -305,6 +308,8 @@ static struct xs_handle *get_handle(const char *connect_to) > if (h->fd == -1) > goto err; > > + h->is_socket = S_ISSOCK(buf.st_mode); > + > XEN_TAILQ_INIT(&h->reply_list); > XEN_TAILQ_INIT(&h->watch_list); > I'd prefer to set h->is_socket above the current if (S_ISSOCK(buf.st_mode)) and then use h->is_socket in this if instead. With that: Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 23/07/2024 10:33 am, Jürgen Groß wrote: > On 18.07.24 18:48, Andrew Cooper wrote: >> It will determine whether to use writev() or sendmsg(). >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Anthony PERARD <anthony.perard@vates.tech> >> CC: Juergen Gross <jgross@suse.com> >> --- >> tools/libs/store/xs.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c >> index 96ea2b192924..f4edeb05f03b 100644 >> --- a/tools/libs/store/xs.c >> +++ b/tools/libs/store/xs.c >> @@ -65,6 +65,9 @@ struct xs_stored_msg { >> struct xs_handle { >> /* Communications channel to xenstore daemon. */ >> int fd; >> + >> + bool is_socket; /* is @fd a file or socket? */ >> + >> Xentoolcore__Active_Handle tc_ah; /* for restrict */ >> /* >> @@ -305,6 +308,8 @@ static struct xs_handle *get_handle(const char >> *connect_to) >> if (h->fd == -1) >> goto err; >> + h->is_socket = S_ISSOCK(buf.st_mode); >> + >> XEN_TAILQ_INIT(&h->reply_list); >> XEN_TAILQ_INIT(&h->watch_list); >> > > I'd prefer to set h->is_socket above the current > > if (S_ISSOCK(buf.st_mode)) > > and then use h->is_socket in this if instead. > > With that: > > Reviewed-by: Juergen Gross <jgross@suse.com> To confirm, you mean like this? @@ -297,7 +300,9 @@ static struct xs_handle *get_handle(const char *connect_to) if (stat(connect_to, &buf) != 0) goto err; - if (S_ISSOCK(buf.st_mode)) + h->is_socket = S_ISSOCK(buf.st_mode); + + if (h->is_socket) h->fd = get_socket(connect_to); else h->fd = get_dev(connect_to);
On 23.07.24 14:07, Andrew Cooper wrote: > On 23/07/2024 10:33 am, Jürgen Groß wrote: >> On 18.07.24 18:48, Andrew Cooper wrote: >>> It will determine whether to use writev() or sendmsg(). >>> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> --- >>> CC: Anthony PERARD <anthony.perard@vates.tech> >>> CC: Juergen Gross <jgross@suse.com> >>> --- >>> tools/libs/store/xs.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c >>> index 96ea2b192924..f4edeb05f03b 100644 >>> --- a/tools/libs/store/xs.c >>> +++ b/tools/libs/store/xs.c >>> @@ -65,6 +65,9 @@ struct xs_stored_msg { >>> struct xs_handle { >>> /* Communications channel to xenstore daemon. */ >>> int fd; >>> + >>> + bool is_socket; /* is @fd a file or socket? */ >>> + >>> Xentoolcore__Active_Handle tc_ah; /* for restrict */ >>> /* >>> @@ -305,6 +308,8 @@ static struct xs_handle *get_handle(const char >>> *connect_to) >>> if (h->fd == -1) >>> goto err; >>> + h->is_socket = S_ISSOCK(buf.st_mode); >>> + >>> XEN_TAILQ_INIT(&h->reply_list); >>> XEN_TAILQ_INIT(&h->watch_list); >>> >> >> I'd prefer to set h->is_socket above the current >> >> if (S_ISSOCK(buf.st_mode)) >> >> and then use h->is_socket in this if instead. >> >> With that: >> >> Reviewed-by: Juergen Gross <jgross@suse.com> > > To confirm, you mean like this? > > @@ -297,7 +300,9 @@ static struct xs_handle *get_handle(const char > *connect_to) > if (stat(connect_to, &buf) != 0) > goto err; > > - if (S_ISSOCK(buf.st_mode)) > + h->is_socket = S_ISSOCK(buf.st_mode); > + > + if (h->is_socket) > h->fd = get_socket(connect_to); > else > h->fd = get_dev(connect_to); > Yes. Juergen
On 2024-07-23 08:08, Jürgen Groß wrote: > On 23.07.24 14:07, Andrew Cooper wrote: >> On 23/07/2024 10:33 am, Jürgen Groß wrote: >>> On 18.07.24 18:48, Andrew Cooper wrote: >>>> It will determine whether to use writev() or sendmsg(). >>>> >>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>>> --- >>>> CC: Anthony PERARD <anthony.perard@vates.tech> >>>> CC: Juergen Gross <jgross@suse.com> >>>> --- >>>> tools/libs/store/xs.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c >>>> index 96ea2b192924..f4edeb05f03b 100644 >>>> --- a/tools/libs/store/xs.c >>>> +++ b/tools/libs/store/xs.c >>>> @@ -65,6 +65,9 @@ struct xs_stored_msg { >>>> struct xs_handle { >>>> /* Communications channel to xenstore daemon. */ >>>> int fd; >>>> + >>>> + bool is_socket; /* is @fd a file or socket? */ >>>> + >>>> Xentoolcore__Active_Handle tc_ah; /* for restrict */ >>>> /* >>>> @@ -305,6 +308,8 @@ static struct xs_handle *get_handle(const char >>>> *connect_to) >>>> if (h->fd == -1) >>>> goto err; >>>> + h->is_socket = S_ISSOCK(buf.st_mode); >>>> + >>>> XEN_TAILQ_INIT(&h->reply_list); >>>> XEN_TAILQ_INIT(&h->watch_list); >>> >>> I'd prefer to set h->is_socket above the current >>> >>> if (S_ISSOCK(buf.st_mode)) >>> >>> and then use h->is_socket in this if instead. >>> >>> With that: >>> >>> Reviewed-by: Juergen Gross <jgross@suse.com> >> >> To confirm, you mean like this? >> >> @@ -297,7 +300,9 @@ static struct xs_handle *get_handle(const char >> *connect_to) >> if (stat(connect_to, &buf) != 0) >> goto err; >> - if (S_ISSOCK(buf.st_mode)) >> + h->is_socket = S_ISSOCK(buf.st_mode); >> + >> + if (h->is_socket) >> h->fd = get_socket(connect_to); >> else >> h->fd = get_dev(connect_to); >> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c index 96ea2b192924..f4edeb05f03b 100644 --- a/tools/libs/store/xs.c +++ b/tools/libs/store/xs.c @@ -65,6 +65,9 @@ struct xs_stored_msg { struct xs_handle { /* Communications channel to xenstore daemon. */ int fd; + + bool is_socket; /* is @fd a file or socket? */ + Xentoolcore__Active_Handle tc_ah; /* for restrict */ /* @@ -305,6 +308,8 @@ static struct xs_handle *get_handle(const char *connect_to) if (h->fd == -1) goto err; + h->is_socket = S_ISSOCK(buf.st_mode); + XEN_TAILQ_INIT(&h->reply_list); XEN_TAILQ_INIT(&h->watch_list);
It will determine whether to use writev() or sendmsg(). Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Juergen Gross <jgross@suse.com> --- tools/libs/store/xs.c | 5 +++++ 1 file changed, 5 insertions(+)