Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Mon, May 14, 2018 at 11:17:55AM -0700, Stephen Hemminger wrote: > On Mon, 14 May 2018 18:14:15 + > Dexuan Cuiwrote: > > > > From: devel On Behalf Of > > > Stephen Hemminger > > > Sent: Sunday, May 13, 2018 10:24 > > > > ... > > > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > > > bool can_sleep) > > > > ... > > > > + hdr = (struct vmbus_channel_message_header > > > > *)buffer; > > > > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > > here. > > > > Yes, it's unnecessary in C, though it's necessary in C++. > > > > I found the patch went into char-misc 4 hours ago, so it looks we may > > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > > > Thanks, > > -- Dexuan > > Kernel developers like to be concise. In fact there is a smatch script that > perodically > gets run and more cleanup patches get sent. It's a Coccinelle script, not Smatch. Coccinelle generates patches automatically so it's a better tool for cleanup than Smatch. I would generate a lot more Smatch information if there was a way to integrate it easily into a code editor. For example, we could highlight unecessary casts or pointer dereferences where Smatch wasn't 100% sure if it was correct. Or you could hover over function name to see what resources it allocates. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
> From: Stephen Hemminger> Sent: Monday, May 14, 2018 11:18 > To: Dexuan Cui > > > ... > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > > here. > > > > Yes, it's unnecessary in C, though it's necessary in C++. > > > > I found the patch went into char-misc 4 hours ago, so it looks we may > > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > > > Thanks, > > -- Dexuan > > Kernel developers like to be concise. In fact there is a smatch script that > perodically gets run and more cleanup patches get sent. I checked the "git log" and confimed you're correct: there are a lot of patches that removed the cast from "void *". :-) Then let me post a small patch for this. -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Mon, 14 May 2018 18:14:15 + Dexuan Cuiwrote: > > From: devel On Behalf Of > > Stephen Hemminger > > Sent: Sunday, May 13, 2018 10:24 > > > ... > > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > > bool can_sleep) > > > ... > > > + hdr = (struct vmbus_channel_message_header *)buffer; > > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > here. > > Yes, it's unnecessary in C, though it's necessary in C++. > > I found the patch went into char-misc 4 hours ago, so it looks we may > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > Thanks, > -- Dexuan Kernel developers like to be concise. In fact there is a smatch script that perodically gets run and more cleanup patches get sent. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
> From: develOn Behalf Of > Stephen Hemminger > Sent: Sunday, May 13, 2018 10:24 > > ... > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > bool can_sleep) > > ... > > + hdr = (struct vmbus_channel_message_header *)buffer; > > Hate to pick o the details, but buffer is void * so cast is not necessary > here. Yes, it's unnecessary in C, though it's necessary in C++. I found the patch went into char-misc 4 hours ago, so it looks we may as well leave it as is. IMHO an explicit cast is not a bad thing. :-) Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Sat, 12 May 2018 02:30:33 -0700 k...@linuxonhyperv.com wrote: > int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep) > { > + struct vmbus_channel_message_header *hdr; > union hv_connection_id conn_id; > int ret = 0; > int retries = 0; > u32 usec = 1; > > conn_id.asu32 = 0; > - conn_id.u.id = VMBUS_MESSAGE_CONNECTION_ID; > + conn_id.u.id = vmbus_connection.msg_conn_id; > > /* >* hv_post_message() can have transient failures because of > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool > can_sleep) > > switch (ret) { > case HV_STATUS_INVALID_CONNECTION_ID: > + /* > + * See vmbus_negotiate_version(): VMBus protocol 5.0 > + * requires that we must use > + * VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate > + * Contact message, but on old hosts that only > + * support VMBus protocol 4.0 or lower, here we get > + * HV_STATUS_INVALID_CONNECTION_ID and we should > + * return an error immediately without retrying. > + */ > + hdr = (struct vmbus_channel_message_header *)buffer; Hate to pick o the details, but buffer is void * so cast is not necessary here. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel