Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110316225421.GC16653@drozd.smutek.pl>
Date: Wed, 16 Mar 2011 23:54:21 +0100
From: Piotr Meyer <aniou@...tek.pl>
To: owl-dev@...ts.openwall.com
Subject: Re: VLANs in Owl way?

On Wed, Mar 16, 2011 at 03:55:10AM +0300, Solar Designer wrote:

> By "just wrote", do you mean you wrote them tonight?

Yes. :) But it wasn't a big deal - code was mostly taken from existing
scripts and I already thought about necessary changes.

> > I will be glad for corrections and suggestions about style, tools
> > and other things (for example: "'-q' should be used rather '>/dev/null'
> > in grep") preferred by Owl team in system scripts.
> 
> Yes, we prefer "-q" over ">/dev/null", so you did the right thing here.

Fixed.

> What are you trying to achieve by using egrep in
> LANG=C egrep -v '(:)'
> 
> Isn't it the same as
> LANG=C fgrep -v ':'

Regular expressions for grep I copy-pasted from old RHEL4 scripts. It
was intentionally - this code works for years and I was afraid of 
errors that can be introduced by ad-hoc, nightly 'fixes'.

Now: yes, now I also don't see reason for use '(:)' instead of ':'
- this was introduced in original patch for vlan support and looks
like nobody takes care about it. 

> In fact, can't you embed that check into the following grep?  Replace
> this one:
> 
> LANG=C egrep -q '(eth|bond)[0-9][0-9]*\.[0-9][0-9]?[0-9]?[0-9]?'
> 
> with:
> 
> LANG=C egrep -q '(eth|bond)[0-9]+\.[0-9]{1,4}($|[^:])'
> 
> (if I got your intent right).

I used suggested format, it's nicer.

> These are patches against files on an installed system.  It's fine for
> the discussion, but when you're done revising the changes and actually
[...]

Attachment contains latest diff (with suggested changes in regexp and
proper support for vlan[0-9]{1,4} format). Is this patch correctly 
formatted?

> Besides changes to the scripts, maybe you need to add documentation?

I add few words to sysconfig.txt, is this enough?

-- 
Piotr 'aniou' Meyer

View attachment "initscripts-5.00-owl-vlan.diff" of type "text/plain" (6585 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.