On 2012-01-27 4:09 PM, Zefir Kurtisi wrote:

On 01/26/2012 04:53 PM, Felix Fietkau wrote:

quoted

On 2012-01-26 4:34 PM, Zefir Kurtisi wrote:
[...]

quoted

+/**
+ * struct pattern_detector - overloading base dfs_pattern_detector
+ *
+ * @exit(): destructor
+ * @add_pulse(): add radar pulse to detector
+ * @num_radar_types: number of different radar types
+ * @last_pulse_ts: time stamp of last valid pulse
+ * @radar_detector_specs: array of radar detection specs
+ * @channel_detectors: list connecting channel_detector elements
+ */
+struct pattern_detector {
+	void (*exit)(struct pattern_detector *_this);
+	enum dfs_detector_result (*add_pulse)
+		(struct pattern_detector *_this, struct pulse_event *pe);
+
+	u8 num_radar_types;
+	u64 last_pulse_ts;
+	struct radar_detector_specs *radar_spec;
+	struct list_head channel_detectors;
+};

To overload it this way is quite fragile. It's better to embed struct
dfs_pattern_detector here. In places where you need to go from the
struct dfs_pattern_detector to this struct, you can then use the
container_of macro, to get at least some form of type safety.

- Felix

Hi Felix,

thanks for taking a look.

Actually, for this initial post we do not need any polymorphism at
all, I could basically make this derived class the interface in the
PATCH to come.

Though (since I am used to this coding style) I'd like to understand
your concern. By 'fragile' you are referring to the risk of base and
derived class diverging, right? If it is that, I see that one is in
trouble if you let that happen. Here with the base class being
defined as external interface holding just two function pointers I
thought the risk is negligible.

Yes, I'm talking about the risk of those two diverging. It may be a
small risk right now, but people tend to forget about things like that
over time, and then it becomes bigger. Also, this is very easy to avoid
in a way that does not change the compiled binary code in any way.

- Felix