Re: drm/i915/gvt: Use common error handling code in shadow_workload_ring_buffer()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Markus, 

I normally keep quiet on threads like this. I half agree with you. Yes, perhaps a reportError function would be a good idea, but it seems that what you are suggesting is trading an inline subroutine call for a spaghetti-code vondition. The goto is used only if you do not have any code that follows, and what you are taking out is a function call that DOES return execution flow to the calling block. You are replacing it with a goto call which does not return execution flow. The risks of process termination during a goto call make it a bad idea. In this case, the subroutine call is the right move. If the reuse of the error message bothers you, perhaps it is a good candidate for a static string definition, so that if it is changed it can be changed in one location in the code. The tradeoff there is that the static string definition will eat a few bytes of memory from the variable storage. Just my perspective. I could be wrong. 

Sent from my iPhone

On Oct 24, 2017, at 9:17 AM, SF Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> wrote:

>>> Add a jump target so that a call of the function "gvt_vgpu_err" is stored
>>> only once at the end of this function implementation.
>>> Replace two calls by goto statements.
>>> 
>>> This issue was detected by using the Coccinelle software.
>> 
>> I don't think this is an issue or an improvement.
> 
> Do you care for the detail how often an error message is stored in the code?
> 
> Regards,
> Markus
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux