From owner-freebsd-hackers@freebsd.org Mon Aug 1 18:36:45 2016 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 43C17BABB7C for ; Mon, 1 Aug 2016 18:36:45 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from venus.codepro.be (venus.codepro.be [IPv6:2a01:4f8:162:1127::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.codepro.be", Issuer "Gandi Standard SSL CA 2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E731818FB for ; Mon, 1 Aug 2016 18:36:44 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from [10.0.2.164] (unknown [IPv6:2a02:1811:2419:4e02:488:21f5:5519:1e0b]) (Authenticated sender: kp) by venus.codepro.be (Postfix) with ESMTPSA id 55CD0C0AF; Mon, 1 Aug 2016 20:36:42 +0200 (CEST) From: "Kristof Provost" To: "Christian Mauderer" Cc: "freebsd-hackers@freebsd.org" Subject: Re: Changes to pfctl to allow easier integration into a library Date: Mon, 01 Aug 2016 20:36:43 +0200 Message-ID: In-Reply-To: References: <25df9fd5-be75-b9ae-aa3a-22abef3bddf0@embedded-brains.de> <0C7EC45D-C3BC-4417-AF77-3ACC027D28B5@FreeBSD.org> MIME-Version: 1.0 X-Mailer: MailMate (2.0BETAr6042) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2016 18:36:45 -0000 On 1 Aug 2016, at 16:32, Christian Mauderer wrote: > Am 01.08.2016 um 16:02 schrieb Kristof Provost: >> On 1 Aug 2016, at 15:03, Christian Mauderer wrote: >>> Can I improve anything to make the patches more acceptable? >>> >> Can you explain why >> 0003-pfctl-Pull-static-variables-out-of-the-function.patch is >> required? >> I’m not sure I see why you need it. >> > I use roughly the following method for the global variables: > > - I put all initialized (zero or value) variables into a special named > linker section. > - In a wrapper around main() I do the following: > o First save the content of the section to a temporary memory space > o Execute the original (mostly unchanged) main() > o After main() finishes, I restore the content of the section > > To simplify a later update to a newer source version, the difference > between the sources we use and the original FreeBSD sources should be > minimal. Therefore my attempt to put the variables into a section is > the > following: > > I create a header file (i.e. pfctl-data.h) that contains a matching > declaration of the global variables but with an added gcc attribute > __attribute__((__section__("my_section_name"))). This header file is > included at the end of the original pfctl.c. > Oh. Ick. Clever, but … ick. I’m not a big fan of this patch, because it makes the code a bit harder to follow, rather than improving things as most of your other patches do. I suspect that something similar can be accomplished with a bit of linker trickery. A first idea is to insert two new symbols (e.g. pf_begin/pf_end) in two separate files, the first before all of the pfctl object files, the second after them. This should let you group the .data section of the pfctl globals, accomplishing what you do here with the __attribute__ attribute. Regards, Kristof