Re: [PATCH 4/6] tools/libxs: Track whether we're using a socket or file

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxx>
>> ---
>> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>> CC: Juergen Gross <jgross@xxxxxxxx>
>> ---
>>   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@xxxxxxxx>

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);