All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-api@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org,
	Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Subject: Re: [PATCH v2 01/10] [media] Move mediabus format definition to a more standard place
Date: Fri, 7 Nov 2014 09:50:25 -0200	[thread overview]
Message-ID: <20141107095025.4dbf6774@concha.lan> (raw)
In-Reply-To: <20141107114358.GB3136@valkosipuli.retiisi.org.uk>

Em Fri, 07 Nov 2014 13:43:59 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

>>  +enum media_bus_format {  
> 
> There's no really a need to keep the definitions inside the enum. It looks a
> little bit confusing to me. That made me realise something I missed
> yesterday.
> 
> There's a difference: the enum in C++ is a different thing than in C, and
> the enum type isn't able to contain any other values than those defined in
> the enumeration.
> 
> So what I propose is the following. Keep enum v4l2_mbus_pixelcode around,
> including the enum values. Define new values for MEDIA_BUS_* equivalents
> using preprocessor macros, as you've done below. Drop the definition of enum
> media_bus_format, and use u32 (or uint32_t) type for the variables.
> 
> This way the enum stays intact for existing C++ applications, and new
> applications will have to use a 32-bit type.

Yeah, enums at the public API is bad, as any change there can potentially
cause C++ apps to break.

-- 

Cheers,
Mauro

WARNING: multiple messages have this Message-ID (diff)
From: mchehab@osg.samsung.com (Mauro Carvalho Chehab)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 01/10] [media] Move mediabus format definition to a more standard place
Date: Fri, 7 Nov 2014 09:50:25 -0200	[thread overview]
Message-ID: <20141107095025.4dbf6774@concha.lan> (raw)
In-Reply-To: <20141107114358.GB3136@valkosipuli.retiisi.org.uk>

Em Fri, 07 Nov 2014 13:43:59 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

>>  +enum media_bus_format {  
> 
> There's no really a need to keep the definitions inside the enum. It looks a
> little bit confusing to me. That made me realise something I missed
> yesterday.
> 
> There's a difference: the enum in C++ is a different thing than in C, and
> the enum type isn't able to contain any other values than those defined in
> the enumeration.
> 
> So what I propose is the following. Keep enum v4l2_mbus_pixelcode around,
> including the enum values. Define new values for MEDIA_BUS_* equivalents
> using preprocessor macros, as you've done below. Drop the definition of enum
> media_bus_format, and use u32 (or uint32_t) type for the variables.
> 
> This way the enum stays intact for existing C++ applications, and new
> applications will have to use a 32-bit type.

Yeah, enums at the public API is bad, as any change there can potentially
cause C++ apps to break.

-- 

Cheers,
Mauro

  reply	other threads:[~2014-11-07 11:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06  9:56 [PATCH v2 00/10] [media] Make mediabus format subsystem neutral Boris Brezillon
2014-11-06  9:56 ` Boris Brezillon
2014-11-06  9:56 ` Boris Brezillon
2014-11-06  9:56 ` [PATCH v2 01/10] [media] Move mediabus format definition to a more standard place Boris Brezillon
2014-11-06  9:56   ` Boris Brezillon
2014-11-06  9:56   ` Boris Brezillon
2014-11-07 11:43   ` Sakari Ailus
2014-11-07 11:43     ` Sakari Ailus
2014-11-07 11:43     ` Sakari Ailus
2014-11-07 11:50     ` Mauro Carvalho Chehab [this message]
2014-11-07 11:50       ` Mauro Carvalho Chehab
     [not found]     ` <20141107114358.GB3136-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
2014-11-07 12:21       ` Boris Brezillon
2014-11-07 12:21         ` Boris Brezillon
2014-11-07 12:21         ` Boris Brezillon
2014-11-07 13:10       ` Hans Verkuil
2014-11-07 13:10         ` Hans Verkuil
2014-11-07 13:10         ` Hans Verkuil
2014-11-06  9:57 ` [PATCH v2 02/10] [media] v4l: Update subdev-formats doc with new MEDIA_BUS_FMT values Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 03/10] [media] Make use of the new media_bus_format definitions Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 04/10] [media] i2c: Make use of media_bus_format enum Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
     [not found] ` <1415267829-4177-1-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-11-06  9:57   ` [PATCH v2 05/10] [media] pci: " Boris Brezillon
2014-11-06  9:57     ` Boris Brezillon
2014-11-06  9:57     ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 06/10] [media] platform: " Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 07/10] [media] usb: " Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 08/10] staging: media: " Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 09/10] gpu: ipu-v3: " Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon
2014-11-06  9:57 ` [PATCH v2 10/10] [media] v4l: Forbid usage of V4L2_MBUS_FMT definitions inside the kernel Boris Brezillon
2014-11-06  9:57   ` Boris Brezillon

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=20141107095025.4dbf6774@concha.lan \
    --to=mchehab@osg.samsung.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=g.liakhovetski@gmx.de \
    --cc=hans.verkuil@cisco.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sakari.ailus@iki.fi \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.