Enhance code readability
Subsystems (3)
cover Enhance code readability
Refactor controls.c for readability
Narcisa Ana Maria Vasile (2):
staging: vc04_services: bcm2835-camera: Remove unnecessary else
instruction
staging: vc04_services: bcm2835-camera: Remove unnecessary variable
.../staging/vc04_services/bcm2835-camera/controls.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/2 staging: vc04_services: bcm2835-camera: Remove unnecessary else instruction
Initialize the variable shutter_speed to avoid unnecessary else branch
--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/controls.c +3 -6
@@ -798,7 +798,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) {int ret =0 ; -int shutter_speed; +int shutter_speed =0 ;struct vchiq_mmal_port *control; v4l2_dbg(0, bcm2835_v4l2_debug, &dev->v4l2_dev,
@@ -815,8 +815,6 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (dev->exposure_mode_user ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; -else - shutter_speed =0 ; v4l2_dbg(0, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: scene mode none: shut_speed %d, exp_mode %d, metering %d\n",
@@ -843,7 +841,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,/* Set up scene mode */ int i;const struct v4l2_mmal_scene_config *scene =NULL ; -int shutter_speed; +int shutter_speed =0 ;enum mmal_parameter_exposuremode exposure_mode;enum mmal_parameter_exposuremeteringmode metering_mode;
@@ -864,8 +862,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; -else - shutter_speed =0 ; + exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode;
Signature · 1 line
1.9.1
Re: [Outreachy kernel] [PATCH 1/2] staging: vc04_services: bcm2835-camera: Remove unnecessary else instruction
Quoted 1 line · Narcisa Ana Maria Vasile
Initialize the variable
shutter_speedto avoid unnecessary else branch
Is the else really bad? With it, one can see the two possible values at once.
julia
Quoted 57 lines
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)drivers/staging/vc04_services/bcm2835-camera/controls.c +3
-7@@ -798,7 +798,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) {int ret =0 ; -int shutter_speed; +int shutter_speed =0 ;struct vchiq_mmal_port *control; v4l2_dbg(0, bcm2835_v4l2_debug, &dev->v4l2_dev,@@ -815,8 +815,6 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (dev->exposure_mode_user ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; -else - shutter_speed =0 ; v4l2_dbg(0, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: scene mode none: shut_speed %d, exp_mode %d, metering %d\n",@@ -843,7 +841,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,/* Set up scene mode */ int i;const struct v4l2_mmal_scene_config *scene =NULL ; -int shutter_speed; +int shutter_speed =0 ;enum mmal_parameter_exposuremode exposure_mode;enum mmal_parameter_exposuremeteringmode metering_mode;@@ -864,8 +862,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; -else - shutter_speed =0 ; + exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode; --1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ff2d51e7327a03defd26b7d3561eb2c154a2fa0c.1488371940.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
PATCH 2/2 staging: vc04_services: bcm2835-camera: Remove unnecessary variable
In reply to Narcisa Ana Maria VasileInstead of using the ret variable, return 0 directly
--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/controls.c +5 -7
@@ -609,18 +609,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,struct v4l2_ctrl *ctrl, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) { - int ret; struct vchiq_mmal_port *encoder_out; dev->capture.encode_bitrate = ctrl->val; encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE ]->output[0 ]; - ret =vchiq_mmal_port_parameter_set (dev->instance, encoder_out, - mmal_ctrl->mmal_id, - &ctrl->val,sizeof (ctrl->val)); - ret =0 ; -return ret; +vchiq_mmal_port_parameter_set (dev->instance, encoder_out, + mmal_ctrl->mmal_id, + &ctrl->val,sizeof (ctrl->val)); +return 0 ; } static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,
@@ -862,7 +860,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; - + exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode;
Signature · 1 line
1.9.1
Re: [Outreachy kernel] [PATCH 2/2] staging: vc04_services: bcm2835-camera: Remove unnecessary variable
Quoted 35 lines · Narcisa Ana Maria Vasile
Instead of using the ret variable, return 0 directly
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)drivers/staging/vc04_services/bcm2835-camera/controls.c +4
-6@@ -609,18 +609,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,struct v4l2_ctrl *ctrl, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) { - int ret; struct vchiq_mmal_port *encoder_out; dev->capture.encode_bitrate = ctrl->val; encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE ]->output[0 ]; - ret =vchiq_mmal_port_parameter_set (dev->instance, encoder_out, - mmal_ctrl->mmal_id, - &ctrl->val,sizeof (ctrl->val)); - ret =0 ; -return ret; +vchiq_mmal_port_parameter_set (dev->instance, encoder_out, + mmal_ctrl->mmal_id, + &ctrl->val,sizeof (ctrl->val)); +return 0 ; } static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,
drivers/staging/vc04_services/bcm2835-camera/controls.c static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev, · line 863
Quoted 5 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +1
-1@@ -862,7 +860,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; - +
Something went wrong here. It's fine to fi the whitespace issue, but it should be a separate patch.
julia
Quoted 13 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +0
-1exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode; --1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/c49c187ff219c61323b25f01ab3f051950ae507a.1488371940.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
Re: [Outreachy kernel] [PATCH 2/2] staging: vc04_services: bcm2835-camera: Remove unnecessary variable
Quoted 48 lines · Julia Lawall
Quoted 35 lines · Narcisa Ana Maria Vasile
Instead of using the ret variable, return 0 directly
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)drivers/staging/vc04_services/bcm2835-camera/controls.c +4
-6@@ -609,18 +609,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,struct v4l2_ctrl *ctrl, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) { - int ret; struct vchiq_mmal_port *encoder_out; dev->capture.encode_bitrate = ctrl->val; encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE ]->output[0 ]; - ret =vchiq_mmal_port_parameter_set (dev->instance, encoder_out, - mmal_ctrl->mmal_id, - &ctrl->val,sizeof (ctrl->val)); - ret =0 ; -return ret; +vchiq_mmal_port_parameter_set (dev->instance, encoder_out, + mmal_ctrl->mmal_id, + &ctrl->val,sizeof (ctrl->val)); +return 0 ; } static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,drivers/staging/vc04_services/bcm2835-camera/controls.c static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev, · line 863
Quoted 5 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +1
-1@@ -862,7 +860,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; - +Something went wrong here. It's fine to fi the whitespace issue, but it should be a separate patch.
Quoted 2 lines
julia
So, should I send a separate patch which solves the problem as in PATCH 2/2 (removes the ret variable and returns directly) or is this an unnecessary change? I'm not sure I understand what the whitespace issue is.
Narcisa
Quoted 13 lines
Quoted 13 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +0
-1exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode; --1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/c49c187ff219c61323b25f01ab3f051950ae507a.1488371940.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
Re: [Outreachy kernel] [PATCH 2/2] staging: vc04_services: bcm2835-camera: Remove unnecessary variable
Quoted 49 lines · Narcisa Ana Maria Vasile
Quoted 48 lines · Julia Lawall
Quoted 35 lines · Narcisa Ana Maria Vasile
Instead of using the ret variable, return 0 directly
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>--- drivers/staging/vc04_services/bcm2835-camera/controls.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)drivers/staging/vc04_services/bcm2835-camera/controls.c +4
-6@@ -609,18 +609,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,struct v4l2_ctrl *ctrl, const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl) { - int ret; struct vchiq_mmal_port *encoder_out; dev->capture.encode_bitrate = ctrl->val; encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE ]->output[0 ]; - ret =vchiq_mmal_port_parameter_set (dev->instance, encoder_out, - mmal_ctrl->mmal_id, - &ctrl->val,sizeof (ctrl->val)); - ret =0 ; -return ret; +vchiq_mmal_port_parameter_set (dev->instance, encoder_out, + mmal_ctrl->mmal_id, + &ctrl->val,sizeof (ctrl->val)); +return 0 ; } static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,drivers/staging/vc04_services/bcm2835-camera/controls.c static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev, · line 863
Quoted 5 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +1
-1@@ -862,7 +860,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,if (scene->exposure_mode ==MMAL_PARAM_EXPOSUREMODE_OFF ) shutter_speed = dev->manual_shutter_speed; - +Something went wrong here. It's fine to fi the whitespace issue, but it should be a separate patch.
Quoted 6 lines
Quoted 2 lines
julia
So, should I send a separate patch which solves the problem as in PATCH 2/2 (removes the ret variable and returns directly) or is this an unnecessary change? I'm not sure I understand what the whitespace issue is.
What you did with ret seems fine. But if you look above there is a - with what looks like an empty line after it, and then a + with what looks like an empty line after it. Actually, the - used to have some tabs after it; they aren't there now so I guess some mailer removed them, but I think they were there in the message that I commented on.
Probably you removed the unnecessary whitespace at some point, then forgot about it and modified the file in another way.
julia
Quoted 23 lines
Narcisa
Quoted 6 lines
Quoted 6 lines
drivers/staging/vc04_services/bcm2835-camera/controls.c +0
-1exposure_mode = scene->exposure_mode; metering_mode = scene->metering_mode; --1.9.1
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/c49c187ff219c61323b25f01ab3f051950ae507a.1488371940.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170301155906.GA20400%40nati-X550JK.
For more options, visit https://groups.google.com/d/optout.