Wireshark-dev: Re: [Wireshark-dev] RFD: Creating subdirectories in epan/dissectors/
On Thu, Aug 30, 2012 at 1:46 PM, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:
> Evan Huus wrote:
>>
>> On Thu, Aug 30, 2012 at 9:45 AM, Graham Bloice
>> <graham.bloice@xxxxxxxxxxxxx> wrote:
>>>>
>>>> -----Original Message-----
>>>> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-
>>>> bounces@xxxxxxxxxxxxx] On Behalf Of Evan Huus
>>>> Sent: 30 August 2012 14:31
>>>> To: Developer support list for Wireshark
>>>> Subject: Re: [Wireshark-dev] RFD: Creating subdirectories in
>>>> epan/dissectors/
>>>>
>>>> On Thu, Aug 30, 2012 at 9:23 AM, Roland Knall <rknall@xxxxxxxxx> wrote:
>>>>>
>>>>> Hi
>>>>>
>>>>> Would you like to enforce a value for the minimum number of subsequent
>>>>> files in the subdirectories?
>>>>
>>>> I would assume you'd need 5 or 6 files at least to make a folder
>>>
>>> worthwhile,
>>>>
>>>> but I don't think that's a hard rule. None of the groups proposed for
>>>
>>> the initial
>>>>
>>>> run have less than 14.
>>>>
>>>>> As I wrote the opensafety package, I would like to split it up a
>>>>> little bit to make it more maintainable, as well as include two new
>>>>> subdissectors, which use the openSAFETY protocol, but are not
>>>>> necessarily part of it.
>>>>
>>>> For the subdissectors, it depends on how tightly they are bound to
>>>> opensafety. If they could theoretically be carried on other protocols as
>>>
>>> well
>>>>
>>>> then they shouldn't be grouped with it, but if they are restricted to
>>>> opensafety (in the sense that they use some special fields or features
>>>
>>> of
>>>>
>>>> opensafety and so actually couldn't be carried on, say, TCP, without
>>>
>>> changing
>>>>
>>>> the protocol), then they can logically be grouped with it. Again,
>>>
>>> probably not
>>>>
>>>> a hard rule, but a good guideline.
>>>>
>>> [Graham Bloice said]
>>>
>>> Some folks have articulated the drawbacks (to them) of making these
>>> changes but I haven't seen any actual advantages listed. Can anyone list
>>> them as they see it?
>>
>>
>> I think it boils down to better structure and scalability. We have an
>> enormous number of files in epan/dissectors/ and I fully expect it to
>> continue to grow. The flat structure is already unwieldy and it's only
>> going to get worse.
>
>
> Unwieldy how? Except for having to know not to do "vi
> epan/dissectors/<tab><tab>" (for fear of too many pages of output) I don't
> find the directory unwieldy.
That's part of it - I still do that accidentally on occasion. The
other part of it is just that the number of files is overwhelming.
It's not necessarily inefficient for the computer, but it is certainly
inefficient for a mental model of the source structure, and it feels
daunting, which is something we want to avoid in order to encourage
new developers. I know when I checked out the source tree for the very
first time I looked at the size of it and had very much a "where do I
start?!" moment.
> I admit I don't use any kind of graphical interface to look in the
> directory... I imagine pointing a file explorer in there might be somewhat
> painful.
It is. I tend not to browse the tree with a GUI, but I have in the
past and it was a nightmare.
>> Logically grouping certain dissectors reduces the number of items at
>> the epan/dissectors/ level. It also makes it much easier to work on a
>> single cluster: if you're working on some enhancements to the
>> bluetooth dissectors, it's much easier to work in
>> epan/dissectors/packet-bluetooth/ than to try and wade through all the
>> other hundreds of files in epan/dissectors/ every time you need to
>> open another bluetooth-related file.
>
>
> Don't you get the same thing by naming the files
> packet-<high_level_protocol>-<subprotocol>, etc.?
To a point, but shell auto-complete is much more efficient if it's a
sub-directory.
>> It should also make the groupings easier for new developers to grok.
>> It's pretty obvious that everything in packet-bluetooth/ must be
>> bluetooth-related, even if the filename is something like
>> packet-hci_h4.c (which is a real bluetooth dissector file, for those
>> who didn't know). *I* know that "hci" stands for Host/Controller
>> Interface which is a bluetooth-related term, but only because I looked
>> it up for the purposes of this email, and I still don't know what the
>> "h4" is. New developers shouldn't have to do that kind of research (or
>> peek at the file header) to get an idea of what a dissector does.
>>
>> Arguably my last point could be fixed by just using better names in
>> epan/dissectors/, but then you end up with names that are
>> uncomfortably long:
>> packet-bluetooth_host-controller-interface_hsomethingsomething4.c
>> anyone?
>
>
> Well, no, but how about packet-bthci_h4.c (to match the naming convention of
> the other bluetooth files)?
That would certainly help, but again it's not immediately obvious to
new devs that packet-bt*.c is bluetooth, and spelling out
packet-bluetooth_*.c gives pretty long names.