git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Make the global packed_git variable static to sha1_file.c.

From
Chris Packham <judge.packham@gmail.com>
Date
Feb 12, 2014, 07:29 UTC
Message-ID
<52FB22F3.8070100@gmail.com>
In-Reply-To
<20140212015727.1D63A403D3@wince.sfo.corp.google.com>
Hi,
On 12/02/14 14:57, Stefan Zager wrote:
Show 39 quoted lines
> From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001
> From: Stefan Zager <szager@chromium.org>
> Date: Mon, 10 Feb 2014 16:55:12 -0800
> Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.
> 
> This is a first step in making the codebase thread-safe.  By and
> large, the operations which might benefit from threading are those
> that work with pack files (e.g., checkout, blame), so the focus of
> this patch is stop leaking the global list of pack files outside of
> sha1_file.c.
> 
> The next step will be to control access to the list of pack files
> with a mutex.  However, that alone is not enough to make pack file
> access thread safe.  Even in a read-only operation, the window list
> associated with each pack file will need to be controlled.
> Additionally, the global counters in sha1_file.c will need to be
> controlled.
> 
> This patch is a pure refactor with no functional changes, so it
> shouldn't require any additional tests.  Adding the actual locks
> will be a functional change, and will require additional tests.
> 
> Signed-off-by: Stefan Zager <szager@chromium.org>
> ---
>  builtin/count-objects.c  |  44 ++++++-----
>  builtin/fsck.c           |  46 +++++++-----
>  builtin/gc.c             |  26 +++----
>  builtin/pack-objects.c   | 188 ++++++++++++++++++++++++++++-------------------
>  builtin/pack-redundant.c |  37 +++++++---
>  cache.h                  |  16 +++-
>  fast-import.c            |   4 +-
>  http-backend.c           |  28 ++++---
>  http-push.c              |   4 +-
>  http-walker.c            |   2 +-
>  pack-revindex.c          |  20 ++---
>  server-info.c            |  35 +++++----
>  sha1_file.c              |  35 ++++++++-
>  sha1_name.c              |  18 ++++-
>  14 files changed, 315 insertions(+), 188 deletions(-)

I'm not really qualified to comment on substance but there are some basic style issues w.r.t. whitespace namely using 4 spaces for indent and mixing tabs/spaces. This might seem pedantic for the first round of a patch but it does put off reviewers.

>From Documentation/CodingGuidelines:
 - We use tabs to indent, and interpret tabs as taking up to
   8 spaces.
Previous: Stefan ZagerNext: Stefan Zager
Message 2 of 5 in “Make the global packed_git variable static to sha1_file.c.”
  1. Make the global packed_git variable static to sha1_file.c.Stefan Zager, Feb 12, 2014
  2. Chris PackhamFeb 12, 2014
  3. Stefan ZagerFeb 12, 2014
  4. David KastrupFeb 12, 2014
  5. Junio C HamanoFeb 12, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.