From: Mauro Carvalho Chehab <mchehab@infradead.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
Brandon Philips <brandon@ifup.org>,
Linux Media Mailing List <linux-media@vger.kernel.org>,
Douglas Landgraf <dougsland@gmail.com>
Subject: Re: [ANNOUNCE] git tree repositories & libv4l
Date: Sat, 23 Jan 2010 22:42:19 -0200 [thread overview]
Message-ID: <4B5B976B.1080309@infradead.org> (raw)
In-Reply-To: <4B5B31A3.9060903@redhat.com>
Hans de Goede wrote:
> Hi,
>
> On 01/21/2010 08:23 AM, Hans Verkuil wrote:
>>>
>
> <snip>
>
>>> Yes, but, as we have also non-c code, some rules there don't apply.
>>> For example the rationale for not using // comments don't apply to c++,
>>> since it is there since the first definition.
>>
>> Most apps are already in 'kernel' style. The main exception being libv4l.
Well... they are "close" to kernel style, but if you run checkpatch over
all files there, I'm sure you'll see lots of violations.
>
> Ack,
>
> which in hind sight may not have been the best choice (I have no personal
> coding style, I'm used to adjusting my style to what ever the project
> I'm working on uses).
>
> Still I would like to keep libv4l as an exception,
If we're adopting one CodingStyle, this should be done for everything, otherwise
it makes no sense to standardize.
> re-indenting it is
> not going to do it any good (I did my best to keep lines within 80
> chars, but moving to tabs as indent will ruin this, and there are quite
> a few nasty nested if cases in there).
The 80 chars limit is nowadays a "soft" simit. There are even some discussions from
people that the resulting code of people sending patches that just avoids the limit
is worse than before.
The idea behind this limit is that lines of code with more than 80 chars is an indication
that the code is more complex than it needs to be. So, the logic there may re-implemented
on a better way.
Just for fun, I ran this indent command (with kernel CodingStyle, except for the 80 chars
limit), over v4l2-apps/libv4l/libv4l2:
indent -npro -kr -i8 -ts8 -sob -ss -ncs -cp1 -l260 *.c
Except for one or two things, the resulting code seems a way better. Yet, as pointed by
Brandon, some manual work is needed to fix some parts of the result, as the indent is not
perfect.
Again for fun, I ran a checkpatch over the resulting code, removing all warnings due
to 80 cols (see enclosed). I think the troubles with the goto labels were introduced by
indent.
---
>From my side, it would be better to use the same CodingStyle also at v4l2-apps,
since this makes easier for people to contribute on both kernel and userspace. Also,
I can read the code faster when it uses the Kernel CodingStyle.
Yet, not all checkpatch warnings apply to userspace, as some are specific to usage
of kernel deprecated functions. That's said, by using an style close to the kernel one,
it is easy to remove from checkpatch the warnings that won't apply to userspace.
Cheers,
Mauro
-
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = SYS_IOCTL(devices[index].fd, VIDIOC_REQBUFS, &req)) < 0) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:102: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = SYS_IOCTL(devices[index].fd, VIDIOC_STREAMON, &type))) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:193: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = SYS_IOCTL(devices[index].fd, VIDIOC_QBUF, &buf))) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:238: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_map_buffers(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:255: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = SYS_IOCTL(devices[index].fd, VIDIOC_DQBUF, buf))) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:259: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' }':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:287: warning: braces {} are not necessary for single statement blocks
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' }':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:338: warning: braces {} are not necessary for single statement blocks
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_request_read_buffers(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:377: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_map_buffers(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:380: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_queue_read_buffers(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:383: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_streamoff(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:395: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if (!v4l2_log_file && (lfname = getenv("LIBV4L2_LOG_FILENAME")))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:493: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if (!(convert = v4lconvert_create(fd)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:518: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1)':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:601: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1)':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:648: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1)':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:718: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_check_buffer_change_ok(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:897: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_check_buffer_change_ok(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:942: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_deactivate_read_stream(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:964: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_deactivate_read_stream(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:984: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_map_buffers(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:989: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_deactivate_read_stream(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1000: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_deactivate_read_stream(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1046: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1)':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1074: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4l2_activate_read_stream(index)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1095: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' leave:':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1113: warning: labels should not be indented
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1 ||':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1125: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' leave:':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1168: warning: labels should not be indented
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1221: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((result = v4lconvert_vidioc_queryctrl(devices[index].convert, &qctrl)))':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1227: ERROR: do not use assignment in if condition
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c: In ' if ((index = v4l2_get_index(fd)) == -1) {':
/home/v4l/master/v4l2-apps/libv4l/libv4l2/libv4l2.c:1248: ERROR: do not use assignment in if condition
next prev parent reply other threads:[~2010-01-24 0:42 UTC|newest]
Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-19 5:34 [ANNOUNCE] git tree repositories Mauro Carvalho Chehab
2010-01-19 7:53 ` Hans Verkuil
2010-01-19 8:10 ` Patrick Boettcher
2010-01-19 11:20 ` Johannes Stezenbach
2010-01-19 11:49 ` Patrick Boettcher
2010-01-19 12:44 ` Mauro Carvalho Chehab
2010-01-20 8:04 ` Markus Heidelberg
2010-01-20 15:11 ` Mauro Carvalho Chehab
2010-01-19 11:55 ` Mauro Carvalho Chehab
2010-01-19 23:38 ` Andy Walls
2010-01-20 1:10 ` hermann pitton
2010-01-20 3:29 ` Andy Walls
2010-01-20 3:29 ` Mauro Carvalho Chehab
2010-01-20 4:08 ` Andy Walls
2010-01-20 15:05 ` Mauro Carvalho Chehab
2010-01-20 11:43 ` BOUWSMA Barry
2010-01-20 10:19 ` BOUWSMA Barry
2010-01-19 11:08 ` Mauro Carvalho Chehab
2010-01-20 8:36 ` [ANNOUNCE] git tree repositories & libv4l Hans de Goede
2010-01-20 8:55 ` Hans Verkuil
2010-01-20 15:41 ` Mauro Carvalho Chehab
2010-01-20 18:50 ` Hans de Goede
2010-01-20 21:07 ` Brandon Philips
2010-01-21 2:07 ` Mauro Carvalho Chehab
2010-01-21 2:46 ` Brandon Philips
2010-01-21 7:34 ` Hans Verkuil
2010-01-21 20:15 ` Mauro Carvalho Chehab
2010-01-23 17:24 ` Hans de Goede
2010-02-22 22:54 ` Brandon Philips
2010-02-22 23:26 ` Hans Verkuil
2010-02-22 23:38 ` Brandon Philips
2010-02-23 0:41 ` Mauro Carvalho Chehab
2010-02-23 1:12 ` Mauro Carvalho Chehab
2010-02-23 8:04 ` Brandon Philips
2010-02-23 13:37 ` Mauro Carvalho Chehab
2010-02-23 9:45 ` Manu Abraham
2010-02-23 11:20 ` Mauro Carvalho Chehab
2010-02-24 2:32 ` hermann pitton
2010-02-23 11:20 ` Manu Abraham
2010-02-23 8:49 ` Hans de Goede
2010-02-23 9:01 ` Hans Verkuil
2010-02-23 9:23 ` Hans de Goede
2010-02-23 9:38 ` Manu Abraham
2010-02-23 12:21 ` Mauro Carvalho Chehab
2010-02-23 13:07 ` Manu Abraham
2010-02-23 13:36 ` Mauro Carvalho Chehab
2010-02-23 14:09 ` Manu Abraham
2010-02-23 12:10 ` Mauro Carvalho Chehab
2010-02-23 11:49 ` Mauro Carvalho Chehab
2010-02-23 15:37 ` Mauro Carvalho Chehab
2010-02-23 15:51 ` Hans de Goede
2010-02-24 0:58 ` Mauro Carvalho Chehab
2010-02-24 12:55 ` Hans de Goede
2010-02-24 13:40 ` Hans Verkuil
2010-02-24 13:42 ` Mauro Carvalho Chehab
2010-02-24 14:32 ` Brandon Philips
2010-02-25 10:52 ` Hans de Goede
2010-02-24 6:05 ` Brandon Philips
2010-02-24 12:44 ` Hans de Goede
2010-02-24 13:26 ` Mauro Carvalho Chehab
2010-02-24 14:29 ` Patrick Boettcher
2010-02-25 4:55 ` Douglas Schilling Landgraf
2010-01-21 7:23 ` Hans Verkuil
2010-01-21 20:04 ` Mauro Carvalho Chehab
2010-01-23 17:28 ` Hans de Goede
2010-01-24 0:42 ` Mauro Carvalho Chehab [this message]
2010-01-25 16:03 ` Hans de Goede
2010-01-20 9:43 ` Laurent Pinchart
2010-01-20 9:54 ` Paulo Assis
2010-01-20 10:43 ` libwecam & uvcdynctrl (was Re: [ANNOUNCE] git tree repositories & libv4l) Hans de Goede
2010-01-19 15:54 ` [ANNOUNCE] git tree repositories Douglas Schilling Landgraf
2010-01-19 8:04 ` Laurent Pinchart
2010-01-19 11:12 ` Mauro Carvalho Chehab
2010-01-19 11:50 ` Laurent Pinchart
2010-01-19 12:36 ` Mauro Carvalho Chehab
2010-01-19 10:04 ` Devin Heitmueller
2010-01-19 11:52 ` Patrick Boettcher
2010-01-19 12:39 ` Mauro Carvalho Chehab
2010-01-19 12:16 ` Mauro Carvalho Chehab
2010-01-19 21:21 ` Bob Cunningham
2010-01-19 22:37 ` hermann pitton
2010-01-20 13:54 ` Laurent Pinchart
2010-01-20 15:00 ` Mauro Carvalho Chehab
2010-01-19 12:56 ` Laurent Pinchart
2010-01-19 13:07 ` Mauro Carvalho Chehab
2010-01-19 13:12 ` Laurent Pinchart
2010-01-19 21:59 ` Johannes Stezenbach
2010-01-21 2:19 ` Mauro Carvalho Chehab
2010-01-21 2:53 ` Trent Piepho
2010-01-21 3:09 ` Mauro Carvalho Chehab
2010-01-20 19:09 ` Guennadi Liakhovetski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B5B976B.1080309@infradead.org \
--to=mchehab@infradead.org \
--cc=brandon@ifup.org \
--cc=dougsland@gmail.com \
--cc=hdegoede@redhat.com \
--cc=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox