From: Dexuan Cui <decui@xxxxxxxxxxxxx>
Date: Wed, 23 Aug 2017 04:52:14 +0000
> +#define VMBUS_PKT_TRAILER (sizeof(u64))
This is not the packet trailer, it's the size of the packet trailer.
Please make this macro name match more accurately what it is.
> + /* Have we sent the zero-length packet (FIN)? */
> + unsigned long fin_sent;
Why does this need to be atomic? Why can't a smaller simpler
mechanism be used to make sure hvs_shutdown() only performs
hvs_send_data() call once on the channel?
> +static inline bool is_valid_srv_id(const uuid_le *id)
> +{
> + return !memcmp(&id->b[4], &srv_id_template.b[4], sizeof(uuid_le) - 4);
> +}
Do not use the inline function attribute in *.c code. Let the
compiler decide.
> +static inline unsigned int get_port_by_srv_id(const uuid_le *svr_id)
Likewise.
> +static inline void hvs_addr_init(struct sockaddr_vm *addr,
Likewise.
> +static inline void hvs_remote_addr_init(struct sockaddr_vm *remote,
> + struct sockaddr_vm *local)
Likewise.
And so on and so forth, please audit this for your entire patch.
> + *((u32 *)&h->vm_srv_id) = vsk->local_addr.svm_port;
> + *((u32 *)&h->host_srv_id) = vsk->remote_addr.svm_port;
There has to be a better way to express this.
And if this is partially initializing vm_srv_id, at a minimum
endianness needs to be taken into account.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
