From patchwork Tue Jul 30 07:24:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13746827 Received: from fout5-smtp.messagingengine.com (fout5-smtp.messagingengine.com [103.168.172.148]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 56E2116B3B7 for ; Tue, 30 Jul 2024 07:24:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.148 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722324277; cv=none; b=jUmsQeX5pkriCvCvqZtm9VvYUwHKkLxDOYNsCvrE8HZImQtEbw7bgtkv3X9CoiM/yJVZWRbONbzsvj4EZshpTWX2dsQlZxg3Ww1cK5EB44ybYE1BWNr6SgQ5zar2rK7bUCx2epMollOXJpD1SQGUwCTFjYUaP3Rt6IMQ0XyMxXQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722324277; c=relaxed/simple; bh=QcsBtmnzLgugl8V3EWXrFSOZ396uz7zoxaXlCNraEf4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QmNSN1J/hENtzOZ6c8Mf0xRILCS8TY4acFFmgUIfgRXPEWoW1IDze+CzmgJo5laeptWLjK+nvACDobSJ5VzQQK2yFPYGkCfEbzDv3Bi//8hi0DSc/vcVPU5JpKkbyGqBno7Hfezxxq40yTePOPY+1DIJlrf+clwTTBQce2KgNeQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=H3hPiibG; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=cgjDksNj; arc=none smtp.client-ip=103.168.172.148 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="H3hPiibG"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="cgjDksNj" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 3C0F1138079D; Tue, 30 Jul 2024 03:24:34 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 30 Jul 2024 03:24:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1722324274; x=1722410674; bh=FsMrbLRZ6+ P841hZ1xX1eBSz3rhqz198w4DZP2+SJGw=; b=H3hPiibGHRswU1pStSeJwyT00i jWdz7k0gyNULTrtNJ3/gztECxoGE0lGD0fcoUJTAVNIT4SGzwbhoZzzHhw634zmz J/RWXAnB1HFJyupR/vm88iUjLoUvF/Gsft+zcv9sV6F+y29GmTYb1glwaHqFshNp 1f0sxpT9N/KyZQPANBz13NFpDKfCIwLCfDtXC+KXJuioXN63hxxI47veeuW3/0Fq VTy1zo8qY7EMUtPQ1xc6EOoc8YgJ5YWk1pCVuPRuZRI7kInSVIY57/b1+tNI6Rti oH7lzOYqD92/EAnGX2j641OTrtVTZ/IB+fplhfytEhHRaw+F5D7H/P/tFSBA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722324274; x=1722410674; bh=FsMrbLRZ6+P841hZ1xX1eBSz3rhq z198w4DZP2+SJGw=; b=cgjDksNjlM8VHM2scQ3AuRoCglpFyPfzX+zaCdUFkW7o R53Nv8WeSLASbjVfqtmIFo92Tn9TkekeXW6qTABCXerOP8Bp6YztbrQ15U9yEfTV qYfVsd/tc2JNwarvs2s3VjUbGYgmF1XCfX1rbkKDyQWTZZA7ij8iJ/viMGqk/xqA OvB+x7P1kwiCL9aZ9X2Tvsa18pp/apsKvaljmd84s/Hp1t8qV1vDXUnoH8CwsEtO XAaZWRe6lmMkExdX9yrUJUwsGiJH5Sz6LoWxgnFrHznsP32Wa+49Qp4CrkqroM8Y oKMSukOyTI8npfevZC6fQMnjrd708Kg5zdqXTHt1ZA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeefgdduvdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepieevkedtgffgleeugfdvledvfedthfegueegfeevjeelueefkeegfeffhefglefg necuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthho pedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 30 Jul 2024 03:24:33 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 2587c0b6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 30 Jul 2024 07:23:04 +0000 (UTC) Date: Tue, 30 Jul 2024 09:24:29 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Karthik Nayak , Junio C Hamano , Phillip Wood Subject: [PATCH v2 0/5] Documentation: some coding guideline updates Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Hi, this is the second version of my patch series that aims to improve our coding guidelines such that we arrive at a more consistent coding style. Changes compared to v1: - Fix clang-format to use a single space to indent preprocessor directives instead of using tabs. Thus, this series is now built with kn/ci-clang-format at 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job, 2024-07-23) merged into v2.46.0. - Adapt the coding guidelines accordingly to also only use a single space for indentation of nested preprocessor directives. - Adopt a proposal by Junio to more clearly spell out the relationship between a subsystem `S`, `struct S` and its functions `S_()`. - Document `S_clear()`-style functions. I have adopted the proposal by Junio hear, where `clear = release + init` with the restriction that `S_init()` must not allocate any resources. - Add another patch on top that makes variable initializers consistent in our coding guidelines. Our style is to add spaces between the curly brace and the initializers (`struct foo bar = { something };`). I think I captured everything that came out of the discussion, but please let me know in case I misinterpreted or forgot anything. Thanks! Patrick Patrick Steinhardt (5): clang-format: fix indentation width for preprocessor directives Documentation: clarify indentation style for C preprocessor directives Documentation: document naming schema for structs and their functions Documentation: document idiomatic function names Documentation: consistently use spaces inside initializers .clang-format | 6 +++-- Documentation/CodingGuidelines | 48 +++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) Range-diff against v1: -: ---------- > 1: c33ad700d6 clang-format: fix indentation width for preprocessor directives 1: 64e0b44993 ! 2: e3baf01234 Documentation: clarify indentation style for C preprocessor directives @@ Metadata ## Commit message ## Documentation: clarify indentation style for C preprocessor directives - There has recently been some discussion around how C preprocessor - directives shall be indented [1]. This discussion was settled towards - indenting after the hash by two spaces [2]. Document it as such. - - [1]: https://lore.kernel.org/all/xmqqwmmm1bw6.fsf@gitster.g/ - [2]: https://lore.kernel.org/all/20240708092317.267915-2-karthik.188@gmail.com/ + In the preceding commit, we have settled on using a single space per + nesting level to indent preprocessor directives. Clarify our coding + guidelines accordingly. Signed-off-by: Patrick Steinhardt @@ Documentation/CodingGuidelines: For C programs: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. -+ - Nested C preprocessor directives are indented after the hash by two -+ spaces per nesting level. ++ - Nested C preprocessor directives are indented after the hash by one ++ space per nesting level. + + #if FOO -+ # include -+ # if BAR -+ # include -+ # endif ++ # include ++ # if BAR ++ # include ++ # endif + #endif + - We try to keep to at most 80 characters per line. 2: 7f07bf1f3b ! 3: 25f73b970d Documentation: document naming schema for struct-related functions @@ Metadata Author: Patrick Steinhardt ## Commit message ## - Documentation: document naming schema for struct-related functions + Documentation: document naming schema for structs and their functions We nowadays have a proper mishmash of struct-related functions that are called `_` (e.g. `clear_prio_queue()`) versus functions @@ Documentation/CodingGuidelines: For C programs: use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb" ./bin-wrappers/git log` (See `wrap-for-bin.sh`.) -+ - Functions that operate on a specific structure and which are used by -+ other subsystems shall be named after the structure. The function -+ name should start with the name of the structure followed by a verb. -+ E.g. ++ - The primary data structure that a subsystem 'S' deals with is called ++ `struct S`. Functions that operate on `struct S` are named ++ `S_()` and should generally receive a pointer to `struct S` as ++ first parameter. E.g. + + struct strbuf; + 3: 5e1de3c315 ! 4: d4ce00303f Documentation: document difference between release and free @@ Metadata Author: Patrick Steinhardt ## Commit message ## - Documentation: document difference between release and free + Documentation: document idiomatic function names We semi-regularly have discussions around whether a function shall be - named `release()` or `free()`. For most of the part we use these two - terminologies quite consistently though: - - - `release()` only frees internal state of a structure, whereas the - structure itself is not free'd. - - - `free()` frees both internal state and the structure itself. + named `S_release()`, `S_clear()` or `S_free()`. Indeed, it may not be + obvious which of these is preferable as we never really defined what + each of these variants means exactly. Carve out a space where we can add idiomatic names for common functions - in our coding guidelines. This space can get extended in the future when - we feel the need to document more idiomatic names. + in our coding guidelines and define each of those functions. Like this, + we can get to a shared understanding of their respective semantics and + can easily point towards our style guide in future discussions such that + our codebase becomes more consistent over time. + + Note that the intent is not to rename all functions which violate these + semantics right away. Rather, the intent is to slowly converge towards a + common style over time. Signed-off-by: Patrick Steinhardt @@ Documentation/CodingGuidelines: For C programs: void reset_strbuf(struct strbuf *buf); + - There are several common idiomatic names for functions performing -+ specific tasks on structures: ++ specific tasks on a structure `S`: + -+ - `_init()` initializes a structure without allocating the ++ - `S_init()` initializes a structure without allocating the + structure itself. + -+ - `_release()` releases a structure's contents without -+ freeing the structure. ++ - `S_release()` releases a structure's contents without freeing the ++ structure. ++ ++ - `S_clear()` is equivalent to `S_release()` followed by `S_init()` ++ such that the structure is directly usable after clearing it. When ++ `S_clear()` is provided, `S_init()` shall not allocate resources ++ that need to be released again. + -+ - `_free()` releases a structure's contents and frees the ++ - `S_free()` releases a structure's contents and frees the + structure. + For Perl programs: -: ---------- > 5: 8789323ac7 Documentation: consistently use spaces inside initializers