video: simplify cx18_get_input() and ivtv_get_input()
PATCH 2/2 video: simplify cx18_get_input() and ivtv_get_input()
From: Márton Németh <[email protected]>
The cx18_get_input() and ivtv_get_input() are called once from the VIDIOC_ENUMINPUT ioctl() and once from the *_log_status() functions. In the first case the struct v4l2_input is already filled with zeros, so doing this again is unnecessary.
The *_log_status() functions are called from VIDIOC_LOG_STATUS ioctl() which is only used for debug purposes, so it is worth to move the filling with zeros to a least frequently used function.
---
drivers/media/video/cx18/cx18-cards.c +1 -2
@@ -320,10 +320,9 @@ int cx18_get_input(struct cx18 *cx, u16"Composite 3" }; -memset (input,0 ,sizeof (*input));if (index >= cx->nof_inputs)return -EINVAL ; - input->index = index; +strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == CX18_CARD_INPUT_VID_TUNER ?
drivers/media/video/cx18/cx18-ioctl.c +4 -0
@@ -712,7 +712,11 @@ static int cx18_log_status(struct file *cx18_read_eeprom (cx, &tv); }cx18_call_i2c_clients (cx,VIDIOC_LOG_STATUS ,NULL ); + +memset (&vidin,0 ,sizeof (vidin)); + vidin.index = cx->active_input;cx18_get_input (cx, cx->active_input, &vidin); +cx18_get_audio_input (cx, cx->audio_input, &audin);CX18_INFO ("Video Input: %s\n" , vidin.name);CX18_INFO ("Audio Input: %s\n" , audin.name);
drivers/media/video/ivtv/ivtv-cards.c +1 -2
@@ -1199,10 +1199,9 @@ int ivtv_get_input(struct ivtv *itv, u16"Composite 3" }; -memset (input,0 ,sizeof (*input));if (index >= itv->nof_inputs)return -EINVAL ; - input->index = index; +strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == IVTV_CARD_INPUT_VID_TUNER ?
drivers/media/video/ivtv/ivtv-ioctl.c +4 -0
@@ -1446,7 +1446,11 @@ static int ivtv_log_status(struct file *ivtv_read_eeprom (itv, &tv); }ivtv_call_i2c_clients (itv,VIDIOC_LOG_STATUS ,NULL ); + +memset (&vidin,0 ,sizeof (vidin)); + vidin.index = itv->active_input;ivtv_get_input (itv, itv->active_input, &vidin); +ivtv_get_audio_input (itv, itv->audio_input, &audin);IVTV_INFO ("Video Input: %s\n" , vidin.name); IVTV_INFO("Audio Input: %s%s\n", audin.name,
Quoted 7 lines · On Monday 13 October 2008 22:54:06 Németh Márton wrote:
From: Márton Németh <[email protected]>
The
cx18_get_input()andivtv_get_input()are called once from theVIDIOC_ENUMINPUTioctl()and once from the*_log_status()functions. In the first case the structv4l2_inputis already filled with zeros, so doing this again is unnecessary.
And in the second case no one cares whether the struct is zeroed. And the same situation is also true for ivtv_get_output().
I would suggest just removing the memset() from ivtv_get_input, ivtv_get_output and cx18_get_input.
Good that you spotted that this memset is no longer needed BTW. I hadn't realized that.
For the record, I NACK this patch, but if you can make a new one that just removes the memsets then I'll sign off on that.
Regards,
Hans
Quoted 76 lines
The
*_log_status()functions are called fromVIDIOC_LOG_STATUSioctl()which is only used for debug purposes, so it is worth to move the filling with zeros to a least frequently used function.Signed-off-by Márton Németh <[email protected]>---
diff -uprlinux-2.6.27.orig/drivers/media/video/cx18/cx18-cards.clinux-2.6.27/drivers/media/video/cx18/cx18-cards.c---linux-2.6.27.orig/drivers/media/video/cx18/cx18-cards.c2008-10-10
00:13:53.000000000 +0200 +++linux-2.6.27/drivers/media/video/cx18/cx18-cards.c2008-10-13
21:27:54.000000000 +0200 @@ -320,10 +320,9 @@ int cx18_get_input(struct cx18 *cx, u16 "Composite 3"drivers/media/video/cx18/cx18-cards.c +1
-2}; -memset (input,0 ,sizeof (*input));if (index >= cx->nof_inputs)return -EINVAL ; - input->index = index; +strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == CX18_CARD_INPUT_VID_TUNER? diff -upr
linux-2.6.27.orig/drivers/media/video/cx18/cx18-ioctl.clinux-2.6.27/drivers/media/video/cx18/cx18-ioctl.c---linux-2.6.27.orig/drivers/media/video/cx18/cx18-ioctl.c2008-10-10
00:13:53.000000000 +0200 +++linux-2.6.27/drivers/media/video/cx18/cx18-ioctl.c2008-10-13
21:28:11.000000000 +0200 @@ -712,7 +712,11 @@ static int cx18_log_status(struct file *cx18_read_eeprom(cx, &tv);drivers/media/video/cx18/cx18-ioctl.c +4
-0}cx18_call_i2c_clients (cx,VIDIOC_LOG_STATUS ,NULL ); + +memset (&vidin,0 ,sizeof (vidin)); + vidin.index = cx->active_input;cx18_get_input (cx, cx->active_input, &vidin); +cx18_get_audio_input (cx, cx->audio_input, &audin);CX18_INFO ("Video Input: %s\n" , vidin.name);CX18_INFO ("Audio Input: %s\n" , audin.name);diff -upr
linux-2.6.27.orig/drivers/media/video/ivtv/ivtv-cards.clinux-2.6.27/drivers/media/video/ivtv/ivtv-cards.c---linux-2.6.27.orig/drivers/media/video/ivtv/ivtv-cards.c2008-10-10
00:13:53.000000000 +0200 +++linux-2.6.27/drivers/media/video/ivtv/ivtv-cards.c2008-10-13
21:22:59.000000000 +0200 @@ -1199,10 +1199,9 @@ int ivtv_get_input(struct ivtv *itv, u16 "Composite 3"drivers/media/video/ivtv/ivtv-cards.c +1
-2}; -memset (input,0 ,sizeof (*input));if (index >= itv->nof_inputs)return -EINVAL ; - input->index = index; +strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == IVTV_CARD_INPUT_VID_TUNER? diff -upr
linux-2.6.27.orig/drivers/media/video/ivtv/ivtv-ioctl.clinux-2.6.27/drivers/media/video/ivtv/ivtv-ioctl.c---linux-2.6.27.orig/drivers/media/video/ivtv/ivtv-ioctl.c2008-10-10
00:13:53.000000000 +0200 +++linux-2.6.27/drivers/media/video/ivtv/ivtv-ioctl.c2008-10-13
21:21:35.000000000 +0200 @@ -1446,7 +1446,11 @@ static int ivtv_log_status(struct file *ivtv_read_eeprom(itv, &tv);drivers/media/video/ivtv/ivtv-ioctl.c +4
-1}ivtv_call_i2c_clients (itv,VIDIOC_LOG_STATUS ,NULL ); + +memset (&vidin,0 ,sizeof (vidin)); + vidin.index = itv->active_input;ivtv_get_input (itv, itv->active_input, &vidin); +ivtv_get_audio_input (itv, itv->audio_input, &audin);IVTV_INFO ("Video Input: %s\n" , vidin.name); IVTV_INFO("Audio Input: %s%s\n", audin.name, --
Quoted 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
Quoted 10 lines · Hans Verkuil
Quoted 7 lines · On Monday 13 October 2008 22:54:06 Németh Márton wrote:
From: Márton Németh <[email protected]>
The
cx18_get_input()andivtv_get_input()are called once from theVIDIOC_ENUMINPUTioctl()and once from the*_log_status()functions. In the first case the structv4l2_inputis already filled with zeros, so doing this again is unnecessary.And in the second case no one cares whether the struct is zeroed. And the same situation is also true for
ivtv_get_output().
Yeah, 'cos there's nothing better than uninitialized fields, like the recent report of a control that returns minimum and maximum values of zero, but a step-size of 9. Why are we optimizing code paths that are not performance critical by uninitializing memory?
--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
Re: [PATCH 2/2] video: simplify cx18_get_input() and ivtv_get_input ()
Quoted 10 lines · On Monday 13 October 2008 23:40:48 Robert William Fuller wrote:
Quoted 9 lines · Hans Verkuil
Quoted 7 lines · On Monday 13 October 2008 22:54:06 Németh Márton wrote:
From: Márton Németh <[email protected]>
The
cx18_get_input()andivtv_get_input()are called once from theVIDIOC_ENUMINPUTioctl()and once from the*_log_status()functions. In the first case the structv4l2_inputis already filled with zeros, so doing this again is unnecessary.
Quoted 7 lines
Quoted 2 lines
And in the second case no one cares whether the struct is zeroed. And the same situation is also true for
ivtv_get_output().Yeah, 'cos there's nothing better than uninitialized fields, like the recent report of a control that returns minimum and maximum values of zero, but a step-size of 9. Why are we optimizing code paths that are not performance critical by uninitializing memory?
It's already initialized to 0 in v4l2-ioctl.c. No need to do it twice.
Regards,
Hans
--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
From: Márton Németh <[email protected]>
The cx18_get_input() and ivtv_get_input() are called once from the VIDIOC_ENUMINPUT ioctl() (see drivers/media/video/v4l2-ioctl.c:1165) and once from the *_log_status() functions. In the first case the struct v4l2_input is already filled with zeros, so doing this again is unnecessary.
The second case is only used for debugging purposes from the VIDIOC_LOG_STATUS ioctl(). Currently only the "name" field is used in the *_log_status() functions.
---
drivers/media/video/cx18/cx18-cards.c +0 -2
@@ -419,10 +419,8 @@ int cx18_get_input(struct cx18 *cx, u16"Composite 3" }; -memset (input,0 ,sizeof (*input));if (index >= cx->nof_inputs)return -EINVAL ; - input->index = index;strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == CX18_CARD_INPUT_VID_TUNER ?
drivers/media/video/ivtv/ivtv-cards.c +0 -2
@@ -1199,10 +1199,8 @@ int ivtv_get_input(struct ivtv *itv, u16"Composite 3" }; -memset (input,0 ,sizeof (*input));if (index >= itv->nof_inputs)return -EINVAL ; - input->index = index;strlcpy (input->name, input_strs[card_input->video_type -1 ],sizeof (input->name)); input->type = (card_input->video_type == IVTV_CARD_INPUT_VID_TUNER ?