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

Re: [PATCH v2 2/5] sha1_file: remove a buggy value setting

From
Wwanghui <hui.wang@windriver.com>
Date
Sep 7, 2011, 09:55 UTC
Message-ID
<4E673F74.9060006@windriver.com>
In-Reply-To
<7vpqjdab76.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 35 quoted lines
> Wang Hui <Hui.Wang@windriver.com> writes:
>
>   
>> From: Hui Wang <Hui.Wang@windriver.com>
>>
>> The ent->base[] is a character array, it has pfxlen characters from
>> position 0 to (pfxlen-1) to contain an alt object dir name, the
>> position pfxlen should be the string terminating character '\0' and
>> is deliberately set to '\0' at the previous code line. The position
>> (pfxlen+1) is given to ent->name.
>>     
>
> Correct. Do you understand why?
>
> We temporarily NUL terminate the ent->base[] so that we can give it to
> is_directory() to see if that is a directory, but the invariants for a
> alternate_object_database instance after it is properly initialized by
> this function are to have:
>
>  - the directory name followed by a slash in the base[] array;
>  - the name pointer pointing at one byte beyond the slash;
>  - name[2] filled with a slash; and
>  - name[41] terminated with NUL.
>
> Later, has_loose_object_nonlocal() calls fill_sha1_path() with the name
> pointer to fill name[0..1, 3..40] with the hexadecimal representation of
> the object name, which would result in base[] array to have the pathname
> for a loose object found in that alternate. The same thing happens in
> open_sha1_file() to read from a loose object in an alternate.
>
> And you are breaking one of the above invariants by removing that slash
> after the directory name. These callers of fill_sha1_path() will see the
> directory name, your NUL, two hex, slash, and 38 hex in base[].
>
>   
Understand now, thanks for your explanation.
> How would the code even work with your patch?
>
>
>   
Previous: Junio C HamanoNext: Junio C Hamano
Message 9 of 11 in “sha1_file: remove only current repository can have relative path limitation”
  1. 0/5 sha1_file: remove only current repository can have relative path limitationWang Hui, Sep 6, 2011
  2. 1/5 sha1_file cleanup: remove redundant variable checkWang Hui, Sep 6, 2011
  3. 2/5 sha1_file: remove a buggy value settingWang Hui, Sep 6, 2011
  4. 3/5 sha1_file: improve directories comparison methodWang Hui, Sep 6, 2011
  5. 4/5 sha1_file: remove relative entries limitationWang Hui, Sep 6, 2011
  6. 5/5 t5710: add testcase for multi-level relative alternatesWang Hui, Sep 6, 2011
  7. Junio C HamanoSep 6, 2011
  8. Junio C HamanoSep 6, 2011
  9. wanghuiSep 7, 2011
  10. Junio C HamanoSep 6, 2011
  11. wanghuiSep 7, 2011

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.