[Bug 165314] Review Request: kismet -- A WLAN detector, sniffer and IDS

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: kismet -- A WLAN detector, sniffer and IDS


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165314





------- Additional Comments From j.w.r.degoede at hhs.nl  2006-04-25 16:18 EST -------
(In reply to comment #20)
> > *Summary must not start with "A ...." drop the "A " .
> 
> I could not find such a rule in the packaging guidelines and from my
> linguistic feeling, a leading "A " sounds better.
> 

You're right its not (no longer) in the guidelines, somehow it got in my head as
being a guideline, I think it has been on the MUST FIX list from reviews of my
own packages too, I don't know the history.


> > *These:
> >  Requires(pre):          %crontabdir
> >  Requires(postun):       %crontabdir
> 
> ... are required resp. the best current way to express:
> 
> * the directory must exist before the package places files into it. Else,
>   when the directory is a symlink (e.g. compare /etc/init.d) in the owning
>   package, you will create oddities.
> 
> * the package must be removed before the directory. Else, the directory
>   can not be removed because it still contains files from 'kismet' and
>   becomes orphaned.  Therefore, a strict '%crontabdir -> kismet' order
>   on installation, and 'kismet -> %crontabdir' order on uninstallation
>   is required. A plain 'Requires:' does not *guarantees* such an order.
> 

I understand / already thought as much, but in essence this means that you
require the crontabs package so why not just replace these 3 lines:
Requires(missingok):	crontabs
Requires(pre):		%crontabdir
Requires(postun):	%crontabdir

with:
Requires:		crontabs

? Anyways this is just a remark, not a blocker. However unfortunatly the
following x86_64 (rawhide) builderror is a blocker:
gpsmap.cc:2217: error: cast from 'void*' to 'int' loses precision

and the 2 suspicious warnings are still there, probably also 64 bit related:
gpsmap.cc:868: warning: comparison is always true due to limited range of data
gpsmap.cc:876: warning: comparison is always true due to limited range of data

I'll take a look at/into this, but my time for Fedora is scarce at the moment.


> 
> > W: kismet-debuginfo dangling-relative-symlink
/usr/src/debug/kismet-2005-08-R1/libpcap-0.9.1-kis/bpf_filter.c
./bpf/net/bpf_filter.c
> > Most of these are OK / have a good reason, so they are ok, it would
> > be nice if you could fix the last one though, but that is not a
> > blocker.
> 
> I think this is a bug in rpm's debuginfo-generator and do not know how
> to solve it cleanly.
> 

Yes it is, I hit it very rescently too, it doesn't handle symlinks properly (it
packages the symlink and  not the link target instead, or both.

> > * Upstream has a much newer version, it would be nice to upgrade to
> >   that version.
> 
> ok, updated to it. But... it causes ugly warnings and I created a lot
> of patches fixing them. They are reported upstream; see
> 
>   http://www.kismetwireless.net/Forum/General/Messages/1145789909.7993579
> 
> I know that they are making the new package different to this one
> reviewed by you, but I think they are required (at least the -packed
> and -alias patches).

I haven't thoroughly re-reviewed this, I did take a peek. You would have been
free under the guidelines to make this version jump after the initial import of
the previous version anyways, thus there is no need for a complete re-review and
this review is taking way too long already so I'm not restarting it from scratch.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.