[Wireshark-dev] FT_STRING, FT_STRINGZPAD, and null padding

[John Thacker]

I don't understand this change, which makes me wonder if I understand
FT_STRINGZPAD vs FT_STRING in general:
https://gitlab.com/wireshark/wireshark/-/commit/a42286524a0e23f5944e988e672a07a5fb395c69


Everything in proto.c and tvbuff.c treats FT_STRING and FT_STRINGZPAD
exactly the same currently, except for checking stray characters. If the
length is given as -1, the length to the end of the tvbuff is used.
Otherwise, the specified length is checked and copied from the tvbuff, and
a NUL is added at the end regardless. The last byte ensures that they both
work even if the buffer is not null terminated.

However, if the string is null terminated somewhere in the middle, all the
bytes after that (presumably padding) are still copied into the return
value, along with the extra '\0'. This doesn't really cause any problems
other than wasting a little bit of memory and time, since the first null
ends the string.

Before that commit, there was absolutely no difference that I could tell.
Now the difference is that if the string is terminated by a NUL somewhere
in the middle before the specified field length, whether or not the bytes
in the padding after that must be null padding. It is FT_STRING that
requires that any padding be zero-padding (and adds an expert warning if
not), whereas FT_STRINGZPAD does not.

While I agree with the commit message that "there are protocols where a
string that's not the full length of the part of the packet for the string
has a null terminator but isn't guaranteed to be fully padded with nulls,"
and that there can be "a separate type for fields where we really *should*
check that the padding is all nulls," I don't see why FT_STRINGZPAD isn't
that latter type. To me, the name FT_STRINGZPAD implies that it is the one
where we really should check that the padding is zero padding.

I would reverse the handling in that commit, and have FT_STRINGZPAD be the
type that does check, and make FT_STRING be the type that does not check
for trailing characters.

The documentation in README.dissector does not really explain the
differences between the use cases, as it seems like FT_STRING can be used
for everything where FT_STRINGZPAD can:

FT_STRING   A string of characters, not necessarily
NULL-terminated, but possibly NULL-padded.This, and
the other string-of-characterstypes, are to be used
for text strings,not raw binary data.
...FT_STRINGZPAD   A NULL-padded string of characters.
   The length is given in the proto_tree_add_item()
call, but may be larger than the length ofthe string,
with extra bytes being NULL padding.This is typically
used for fixed-length fieldsthat contain a string
value that might be shorterthan the fixed length.

John Thacker
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Managing Gitlab Issues

[Uli Heilmeier]

> Am 04.09.2020 um 14:59 schrieb Dario Lombardo :
> 
> On Fri, Sep 4, 2020 at 1:12 PM Uli Heilmeier  > wrote:
> Hi list,
> 
> I’ve tried to update the instructions to report an issue (fka bug) in the 
> wiki [1].
> 
> There are some things we need to sort out. (Maybe this has already been done 
> on the core list.)
> 
> * Do we want to have labels to mark the status of an issue? With Bugzilla we 
> had Confirmed, Incomplete, In Progress etc. I would like to have labels for 
> status additionally to the existing labels.
> 
> Personally I don't like this. Labels are best to mark the issues as belonging 
> to a component, a version and so on. I don't see the need to stick with 
> gerrit's model, unless we really like it. Again, personally I liked gerrit's 
> labels, but I won't miss it, and I think that model can be left behind.
>  

I'm sorry I was unclear. I’m talking about the Status field of Bugzilla. Not 
Gerrit labels. With Bugzilla a new bug had (normally) the status ‚Unconfirmed‘ 
at the beginning. When someone was able to reproduce the issue the bug had the 
status ‚Confirmed‘. If something was missing (a capture file for example) the 
status was ‚Incomplete‘. When someone was working on a fix the status was ‚In 
Progress‘.
This is currently missing with Gitlab Issues. There is only ‚Open‘ and ‚Closed‘.

Therefore I suggest we use labels to mark the status of an issue.

> 
> * Who should be able to edit issues (e.g. adding labels)? According to Gitlab 
> documentation [2] the Reporter role can do it. However the Reporter role also 
> allows to see issues which are marked as confidentially.
> 
> I'm not sure I got your point. Just a few people (not including the whole 
> core-dev group) have internal access to the project. Just Gerald and a few 
> have. Other code-devs are in a group allowed to merge, but they're not 
> project members. How would you leverage the confidentiality feature?

With Bugzilla I (as a normal contributor) was able to set the status of a bug 
(not limited to my own created ones). Also I was able to correct the 
classification etc. At the moment I’m not able to do this with Gitlab issues. 
When we can have a new group to edit/label issues I’m totally fine. 

>  
> 
> * It would make sense to have templates for issues [3]. Has anyone already 
> prepared this? Otherwise I will create one and submit a MR?
> 
> This would help for sure. Please submit a MR for that.

Will do so.

___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Managing Gitlab Issues

[Dario Lombardo]

On Fri, Sep 4, 2020 at 1:12 PM Uli Heilmeier  wrote:

> Hi list,
>
> I’ve tried to update the instructions to report an issue (fka bug) in the
> wiki [1].
>
> There are some things we need to sort out. (Maybe this has already been
> done on the core list.)
>
> * Do we want to have labels to mark the status of an issue? With Bugzilla
> we had Confirmed, Incomplete, In Progress etc. I would like to have labels
> for status additionally to the existing labels.
>

Personally I don't like this. Labels are best to mark the issues as
belonging to a component, a version and so on. I don't see the need to
stick with gerrit's model, unless we really like it. Again, personally I
liked gerrit's labels, but I won't miss it, and I think that model can be
left behind.


>
> * Who should be able to edit issues (e.g. adding labels)? According to
> Gitlab documentation [2] the Reporter role can do it. However the Reporter
> role also allows to see issues which are marked as confidentially.
>

I'm not sure I got your point. Just a few people (not including the whole
core-dev group) have internal access to the project. Just Gerald and a few
have. Other code-devs are in a group allowed to merge, but they're not
project members. How would you leverage the confidentiality feature?


>
> * It would make sense to have templates for issues [3]. Has anyone already
> prepared this? Otherwise I will create one and submit a MR?
>

This would help for sure. Please submit a MR for that.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] Managing Gitlab Issues

[Uli Heilmeier]

Hi list,

I’ve tried to update the instructions to report an issue (fka bug) in the wiki 
[1].

There are some things we need to sort out. (Maybe this has already been done on 
the core list.)

* Do we want to have labels to mark the status of an issue? With Bugzilla we 
had Confirmed, Incomplete, In Progress etc. I would like to have labels for 
status additionally to the existing labels.

* Who should be able to edit issues (e.g. adding labels)? According to Gitlab 
documentation [2] the Reporter role can do it. However the Reporter role also 
allows to see issues which are marked as confidentially.

* It would make sense to have templates for issues [3]. Has anyone already 
prepared this? Otherwise I will create one and submit a MR?

Cheers

[1] https://gitlab.com/wireshark/wireshark/-/wikis/ReportingBugs
[2] https://docs.gitlab.com/ee/user/permissions.html
[3] https://docs.gitlab.com/ee/user/project/description_templates.html
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Winflexbison update in choco

[Dario Lombardo]

Other users are reporting an issue in the new version of the choco package.
Read the discussion on

https://chocolatey.org/packages/winflexbison3

I will try to downgrade the package version, until the package gets fixed.
It looks like the new version basically doesn't install. This isn't spotted
unless you install from scratch (that's what CIs do).

On Wed, Sep 2, 2020 at 3:00 PM Dario Lombardo  wrote:

> The build is actually done by the github actions builder. A brand new dir
> gets created every time.
>
>
> https://github.com/crondaemon/wireshark/actions?query=workflow%3A%22Build+Windows%22+branch%3Amaster
>
> On Wed, Sep 2, 2020 at 2:53 PM Graham Bloice 
> wrote:
>
>>
>> On Wed, 2 Sep 2020 at 13:43, Dario Lombardo  wrote:
>>
>>> Hi
>>> Some days ago (30th aug), the choco package winflexbison3 was updated.
>>> Since then, my cmake can't find the LEX_EXECUTABLE as well as the
>>> YACC_EXECUTABLE.
>>> Any idea on what happened?
>>> I've fixed the builds by setting LEX_ and YACC_EXECUTABLE in cmake, but
>>> I don't know whether this is the correct fix or not.
>>> Dario.
>>>
>>>
>> Either delete CMakeCache.txt (to discover all required libs etc. again),
>> or edit it to remove the invalid path references and then CMake will search
>> and find them again.
>>
>> --
>> Graham Bloice
>>
>> ___
>> Sent via:Wireshark-dev mailing list 
>> Archives:https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>>  mailto:wireshark-dev-requ...@wireshark.org
>> ?subject=unsubscribe
>
>
>
> --
>
> Naima is online.
>
>

-- 

Naima is online.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe