Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <24df021f5e30c51e2d9fff8533ff5591d37d2d2d.camel@gmail.com>
Subject: Re: [PATCH 1/2] Support python 3 in utils/key2pub.py.
From:   jtojnar@gmail.com
To:     ahmed.taahir@gmail.com, linux-wireless@vger.kernel.org
Date:   Wed, 25 Dec 2019 17:19:01 +0100
In-Reply-To: <1437542484-23409-2-git-send-email-ahmed.taahir@gmail.com> (sfid-20150722_072142_438085_5E3B829E)
References: <1437542484-23409-1-git-send-email-ahmed.taahir@gmail.com>
         <1437542484-23409-2-git-send-email-ahmed.taahir@gmail.com>
         (sfid-20150722_072142_438085_5E3B829E)
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.2 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Wed, 2015-07-22 at 00:21 -0500, ahmed.taahir@gmail.com wrote:
> From: Taahir Ahmed <ahmed.taahir@gmail.com>
> 
> utils/key2pub.py can now be run under either python 2.7 or python
> 3.x.
> This required some minor syntactical changes as well as switching
> from
> M2Crypto to pycrypto, since M2Crypto doesn't support python 3.x.
[...]
>  CFLAGS += -O2 -fpic
>  CFLAGS += -std=gnu99 -Wall -Werror -pedantic
>  CFLAGS += -Wall -g
> @@ -42,13 +46,13 @@ ifeq ($(USE_OPENSSL),1)
>  CFLAGS += -DUSE_OPENSSL -DPUBKEY_DIR=\"$(RUNTIME_PUBKEY_DIR)\" `pkg-
> config --cflags openssl`
>  LDLIBS += `pkg-config --libs openssl`
>  
> -$(LIBREG): keys-ssl.c
> +$(LIBREG): keys-ssl.h
>  
>  else
>  CFLAGS += -DUSE_GCRYPT
>  LDLIBS += -lgcrypt
>  
> -$(LIBREG): keys-gcrypt.c
> +$(LIBREG): keys-gcrypt.h

Changing the file names to headers appears to install them. We probably
do not want to do that either.

[...]
> -    modes[mode][0](output, 'e_%d' % idx, key.e[4:])
> -    modes[mode][0](output, 'n_%d' % idx, key.n[4:])
> +    key_contents = io.open(f, 'rb').read()
> +    key = RSA.importKey(key_contents)

It is a good practice to close opened files. It would be even better to
use context manager (i.e. `with io.open(f, 'rb') as key_file:`) since
it will close the file automatically.

Other than this the patches look good, crda builds with both Python 2
and Python 3 with the patch and produce the same keys-gcrypt.h files.
Comparing to the keys-gcrypt.c, apart from the trivial formatting
changes, the ul suffixes for numbers are the only difference.

I was not able to build crda with USE_OPENSSL=1 with or without this
patch due to a large number of compilation errors. However, the
differences between keys-ssl.c and keys-ssl.h are equivalent to the
grypt ones. Also the addition of ul suffixes fixed the overflow errors, so definitely an improvement in this case as well.