On 01/26/2012 04:53 PM, Felix Fietkau wrote:
> On 2012-01-26 4:34 PM, Zefir Kurtisi wrote:
> [...]
>> +/**
>> + * 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.
But generally you're right of course. I'll take it into account for the PATCH.
Thanks
Zefir
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
