[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: wireshark templates addition



Hi Justin,

Thanks for the review!
I applied the patch and will be available in wireshark 2.4.0-1 after
FTP Masters accept it.

Cheers,
Balint

On Mon, Jul 31, 2017 at 7:58 PM, Justin B Rye <justin.byam.rye@gmail.com> wrote:
> Balint Reczey wrote:
>> Please review the proposed additions to wireshark's templates.
>
> I've attached an attempt at my usual revised-version-and-diff, but my
> apologies if it's malformed.
>
> [...]
>> +
>> +Template: wireshark-common/addgroup-failed
>> +Type: note
>
> If this is an error message, DevRef recommends "Type: error" - see
> https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.html#s6.5.2
>
>> +_Description: Creating the wireshark system group failed
>> + The wireshark group does not exist and creating it failed which prevents
>> + configuring Wireshark for capturing traffic as an unprivileged user.
>> + .
>> + Please create the wireshark system group and try configuring
>> + wireshark-common again.
>
> That first paragraph's a bit awkwardly phrased (but I wouldn't have
> bothered with it if I wasn't already suggesting changes).  Try:
>
>     The wireshark group does not exist, and creating it failed, so
>     Wireshark cannot be configured to capture traffic as an unprivileged
>     user.
>
>> +
>> +Template: wireshark-common/group-is-user-group
>> +Type: note
>> +_Description: The wireshark group is a system group
>> + The wireshark group is an existing user group, but the preferred
>> + configuration is creating it as a system group.
>
> This sounds as if it's saying that something "is creating" it right
> now.  What you mean is
>
>     The wireshark group exists as a user group, but the preferred
>     configuration is for it to be created as a system group.
>
>> + .
>> + Purging wireshark-common will not remove the wireshark group as a result,
>> + but everything else should work properly.
>
> Ambiguously placed "as a result".
>
>     As a result, purging wireshark-common will not remove the wireshark
>     group, but everything else should work properly.
>
>> +
>> +Template: wireshark-common/setcap-failed
>> +Type: note
>
> Possibly
>    Type: error
>
>> +_Description: Setting capabilities for dumpcap using Linux Capabilities failed
>
> This is a bit long (DevRef recommends <= 50 characters), mainly
> because it mentions capabilities twice.
>
>    _Description: Setting capabilities for dumpcap failed
>
> (Is there any reason to capitalise "Capabilities"?)
>
>> + The set-user-id bit is set for the dumpcap binary to gain packet capturing
>> + privileges.
>
> This seems to skip the stage of describing what has gone wrong and
> jump straight to an explanation of what has been done instead.  If I'm
> understanding correctly, I'd suggest:
>
>     The attempt to use Linux capabilities to grant packet-capturing
>     privileges to the dumpcap binary failed. Instead, it has had the
>     set-user-id bit set.
>
> (I hope I'm the only one who keeps getting dumpcap muddled up with
> getcap...)
>
>> +
>> +Template: wireshark-common/group-removal-failed
>> +Type: note
>
> Again
>    Type: error
>
>> +_Description: Removal of the wireshark group failed
>> + When the wireshark-common package is configured to allow
>> + non-superusers to capture packets the postinst script of
>> + wireshark-common creates the wireshark group as a system group.
>> + .
>> + However, on this system the wireshark group is a user group instead of
>> + being a system group thus purging wireshark-common did not remove it.
>
> Another low-priority rephrasing:
>     being a system group, so purging wireshark-common did not remove it.
>
>> + .
>> + Please remove the group manually in case it is not needed anymore.
>
> For most native English-speakers, "in case" means "lest", not "if".
> And then usage of "anymore" varies trickily between dialects.  I'd
> suggest:
>
>     If the group is no longer needed, please remove it manually.
>
> --
> JBR     with qualifications in linguistics, experience as a Debian
>         sysadmin, and probably no clue about this particular package



-- 
Balint Reczey
Debian & Ubuntu Developer


Reply to: