On 08/10/2014 05:57 AM, Gonglei wrote:
+ /* can return a peer or the local client */
+ peer = ivshmem_client_search_peer(client, peer_id);
+
+ /* delete peer */
+ if (fd == -1) {
+
Maybe the above check should be moved before getting the peer.
And the next check peer is extra.
We always need to know the peer, either for a deletion, creation or update.
+ if (peer == NULL || peer == &client->local) {
+ debug_log(client, "receive delete for invalid peer %ld",
Missing '\n' ?
Ok.
peer_id);
+ return -1;
+ }
+
+ debug_log(client, "delete peer id = %ld\n", peer_id);
+ free_peer(client, peer);
+ return 0;
+ }
+
+ /* new peer */
+ if (peer == NULL) {
+ peer = malloc(sizeof(*peer));
g_malloc0 ?.
Ok, replaced malloc/free with g_malloc/g_free in client and server.
+ client->notif_cb = notif_cb;
+ client->notif_arg = notif_arg;
+ client->verbose = verbose;
Missing client->sock_fd = -1; ?
Ok.
+
+ /* first, we expect our index + a fd == -1 */
+ if (read_one_msg(client, &client->local.id, &fd) < 0 ||
+ client->local.id < 0 || fd != -1) {
Why not fd < 0 ?
Because the server will send us an id and a fd == -1 see
ivshmem-server.c send_initial_info().
+ debug_log(client, "cannot read from server\n");
+ close(client->sock_fd);
+ client->sock_fd = -1;
+ return -1;
+ }
+ debug_log(client, "our_id=%ld\n", client->local.id);
+
+ /* now, we expect shared mem fd + a -1 index, note that shm fd
+ * is not used */
+ if (read_one_msg(client, &tmp, &fd) < 0 ||
+ tmp != -1 || fd < 0) {
+ debug_log(client, "cannot read from server (2)\n");
+ close(client->sock_fd);
+ client->sock_fd = -1;
+ return -1;
I think the error logic handle can move the end of this function, reducing
duplicated code. Something like this:
goto err;
}
err:
debug_log(client, "cannot read from server (2)\n");
close(client->sock_fd);
client->sock_fd = -1;
return -1;
Ok, I also updated the server.
+ int fd;
+
+ if (vector > peer->vectors_count) {
Maybe if (vector >= peer->vectors_count) , otherwise the
peer->vectors[] array bounds.
Oh yes, good catch.
It should not happen, at the moment, but it is wrong, indeed.
+/* send a notification to all vectors of a peer */
+int
+ivshmem_client_notify_all_vects(const struct ivshmem_client *client,
+ const struct ivshmem_client_peer
*peer)
+{
+ unsigned vector;
+ int ret = 0;
+
+ for (vector = 0; vector < peer->vectors_count; vector++) {
+ if (ivshmem_client_notify(client, peer, vector) < 0) {
+ ret = -1;
The ret's value will be covered when multi clients failed. Do we need
store the failed status for server?.
It indicates that we could not notify *all* clients.
Thanks Gonglei.
--
David Marchand
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
