[libcamera-devel] [PATCH] libcamera: Declare functions before variables in class definitions

[Umang Jain]

Hi Laurent,

On Tue, 2020-05-26 at 06:26 +0300, Laurent Pinchart wrote:
> The preferred coding style in libcamera is to declare private
> functions
> before private variables in class definitions. This rule isn't
> followed
> by some of the internal classes. Update them accordingly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  .../internal/device_enumerator_udev.h         | 16 ++++++------
>  .../internal/event_dispatcher_poll.h          | 12 ++++-----
>  include/libcamera/internal/ipa_manager.h      |  4 +--
>  include/libcamera/internal/ipa_module.h       |  4 +--
>  include/libcamera/internal/media_device.h     | 26 +++++++++------
> ----
>  include/libcamera/internal/media_object.h     |  4 +--
>  6 files changed, 33 insertions(+), 33 deletions(-)
> 
I spotted one more place that is not included in this patch, which
is : include/libcamera/ipa_context_wrapper.h

Apart from that:
Reviewed-by: Umang Jain <email at uajain.com>
> diff --git a/include/libcamera/internal/device_enumerator_udev.h
> b/include/libcamera/internal/device_enumerator_udev.h
> index fdaa20968ef0..10d17ed2abac 100644
> --- a/include/libcamera/internal/device_enumerator_udev.h
> +++ b/include/libcamera/internal/device_enumerator_udev.h
> @@ -36,10 +36,6 @@ public:
>  	int enumerate() final;
>  
>  private:
> -	struct udev *udev_;
> -	struct udev_monitor *monitor_;
> -	EventNotifier *notifier_;
> -
>  	using DependencyMap = std::map<dev_t, std::list<MediaEntity
> *>>;
>  
>  	struct MediaDeviceDeps {
> @@ -58,16 +54,20 @@ private:
>  		DependencyMap deps_;
>  	};
>  
> -	std::set<dev_t> orphans_;
> -	std::list<MediaDeviceDeps> pending_;
> -	std::map<dev_t, MediaDeviceDeps *> devMap_;
> -
>  	int addUdevDevice(struct udev_device *dev);
>  	int populateMediaDevice(MediaDevice *media, DependencyMap
> *deps);
>  	std::string lookupDeviceNode(dev_t devnum);
>  
>  	int addV4L2Device(dev_t devnum);
>  	void udevNotify(EventNotifier *notifier);
> +
> +	struct udev *udev_;
> +	struct udev_monitor *monitor_;
> +	EventNotifier *notifier_;
> +
> +	std::set<dev_t> orphans_;
> +	std::list<MediaDeviceDeps> pending_;
> +	std::map<dev_t, MediaDeviceDeps *> devMap_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/event_dispatcher_poll.h
> b/include/libcamera/internal/event_dispatcher_poll.h
> index 1f0738617425..3c9099660c4d 100644
> --- a/include/libcamera/internal/event_dispatcher_poll.h
> +++ b/include/libcamera/internal/event_dispatcher_poll.h
> @@ -41,16 +41,16 @@ private:
>  		EventNotifier *notifiers[3];
>  	};
>  
> -	std::map<int, EventNotifierSetPoll> notifiers_;
> -	std::list<Timer *> timers_;
> -	int eventfd_;
> -
> -	bool processingEvents_;
> -
>  	int poll(std::vector<struct pollfd> *pollfds);
>  	void processInterrupt(const struct pollfd &pfd);
>  	void processNotifiers(const std::vector<struct pollfd>
> &pollfds);
>  	void processTimers();
> +
> +	std::map<int, EventNotifierSetPoll> notifiers_;
> +	std::list<Timer *> timers_;
> +	int eventfd_;
> +
> +	bool processingEvents_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/ipa_manager.h
> b/include/libcamera/internal/ipa_manager.h
> index 2412d75746ac..16d742918cf2 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -29,8 +29,6 @@ public:
>  					    uint32_t minVersion);
>  
>  private:
> -	std::vector<IPAModule *> modules_;
> -
>  	IPAManager();
>  	~IPAManager();
>  
> @@ -40,6 +38,8 @@ private:
>  
>  	bool isSignatureValid(IPAModule *ipa) const;
>  
> +	std::vector<IPAModule *> modules_;
> +
>  #if HAVE_IPA_PUBKEY
>  	static const uint8_t publicKeyData_[];
>  	static const PubKey pubKey_;
> diff --git a/include/libcamera/internal/ipa_module.h
> b/include/libcamera/internal/ipa_module.h
> index 5b54cb31a48a..788e31d8bf03 100644
> --- a/include/libcamera/internal/ipa_module.h
> +++ b/include/libcamera/internal/ipa_module.h
> @@ -42,6 +42,8 @@ protected:
>  	std::string logPrefix() const override;
>  
>  private:
> +	int loadIPAModuleInfo();
> +
>  	struct IPAModuleInfo info_;
>  	std::vector<uint8_t> signature_;
>  
> @@ -52,8 +54,6 @@ private:
>  	void *dlHandle_;
>  	typedef struct ipa_context *(*IPAIntfFactory)(void);
>  	IPAIntfFactory ipaCreate_;
> -
> -	int loadIPAModuleInfo();
>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/media_device.h
> b/include/libcamera/internal/media_device.h
> index 9fe76c514b17..19af059d9291 100644
> --- a/include/libcamera/internal/media_device.h
> +++ b/include/libcamera/internal/media_device.h
> @@ -58,26 +58,13 @@ protected:
>  	std::string logPrefix() const;
>  
>  private:
> -	std::string driver_;
> -	std::string deviceNode_;
> -	std::string model_;
> -	unsigned int version_;
> -
> -	int fd_;
> -	bool valid_;
> -	bool acquired_;
> -	bool lockOwner_;
> -
>  	int open();
>  	void close();
>  
> -	std::map<unsigned int, MediaObject *> objects_;
>  	MediaObject *object(unsigned int id);
>  	bool addObject(MediaObject *object);
>  	void clear();
>  
> -	std::vector<MediaEntity *> entities_;
> -
>  	struct media_v2_interface *findInterface(const struct
> media_v2_topology &topology,
>  						 unsigned int
> entityId);
>  	bool populateEntities(const struct media_v2_topology
> &topology);
> @@ -87,6 +74,19 @@ private:
>  
>  	friend int MediaLink::setEnabled(bool enable);
>  	int setupLink(const MediaLink *link, unsigned int flags);
> +
> +	std::string driver_;
> +	std::string deviceNode_;
> +	std::string model_;
> +	unsigned int version_;
> +
> +	int fd_;
> +	bool valid_;
> +	bool acquired_;
> +	bool lockOwner_;
> +
> +	std::map<unsigned int, MediaObject *> objects_;
> +	std::vector<MediaEntity *> entities_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/media_object.h
> b/include/libcamera/internal/media_object.h
> index 748eafdc880b..e8f2f27ce99e 100644
> --- a/include/libcamera/internal/media_object.h
> +++ b/include/libcamera/internal/media_object.h
> @@ -107,6 +107,8 @@ private:
>  	MediaEntity(const MediaEntity &) = delete;
>  	~MediaEntity();
>  
> +	void addPad(MediaPad *pad);
> +
>  	std::string name_;
>  	unsigned int function_;
>  	unsigned int flags_;
> @@ -115,8 +117,6 @@ private:
>  	unsigned int minor_;
>  
>  	std::vector<MediaPad *> pads_;
> -
> -	void addPad(MediaPad *pad);
>  };
>  
>  } /* namespace libcamera */