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

Re: [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 9, 2017, 21:19 UTC
Message-ID
<xmqq1sv7oyei.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<111d663c0fd3e9669e7c28537f581833488ca4a6.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 5 quoted lines
> The old practice of storing the empty string in this member for the main
> repository was a holdover from before 00eebe3 (refs: create a base class
> "ref_store" for files_ref_store, 2016-09-04), when the submodule was
> stored in a flex array at the end of `struct files_ref_store`. Storing
> NULL for this case is more idiomatic and a tiny bit less code.

Yes. I noticed this bit in 3/5 and wondered about it, knowing this step comes next:

Show 15 quoted lines
>  struct ref_store *ref_store_init(const char *submodule)
>  {
>  	const char *be_name = "files";
>  	struct ref_storage_be *be = find_ref_storage_backend(be_name);
> +	struct ref_store *refs;
>  
>  	if (!be)
>  		die("BUG: reference backend %s is unknown", be_name);
>  
>  	if (!submodule || !*submodule)
> -		return be->init(NULL);
> +		refs = be->init(NULL);
>  	else
> -		return be->init(submodule);
> +		refs = be->init(submodule);
Can't we also lose this "if !*submodule, turn it to NULL"?
Show 10 quoted lines
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
>  	struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
>  	struct ref_store *ref_store = (struct ref_store *)refs;
>  
> -	base_ref_store_init(ref_store, &refs_be_files, submodule);
> +	base_ref_store_init(ref_store, &refs_be_files);
>  
>  	refs->submodule = submodule ? xstrdup(submodule) : "";
Also, can't we use xstrdup_or_null(submodule) with this step?
Previous: Stefan BellerNext: Stefan Beller
Message 16 of 28 in “Store submodules in a hash, not a linked list”
  1. 0/5 Store submodules in a hash, not a linked listMichael Haggerty, Feb 9, 2017
  2. 3/5 register_ref_store(): new functionMichael Haggerty, Feb 9, 2017
  3. Stefan BellerFeb 9, 2017
  4. Michael HaggertyFeb 9, 2017
  5. Junio C HamanoFeb 9, 2017
  6. 1/5 refs: store submodule ref stores in a hashmapMichael Haggerty, Feb 9, 2017
  7. Stefan BellerFeb 9, 2017
  8. Michael HaggertyFeb 9, 2017
  9. Stefan BellerFeb 9, 2017
  10. Jeff KingFeb 9, 2017
  11. Junio C HamanoFeb 9, 2017
  12. Jeff KingFeb 10, 2017
  13. Michael HaggertyFeb 10, 2017
  14. 4/5 files_ref_store::submodule: use NULL for the main repositoryMichael Haggerty, Feb 9, 2017
  15. Stefan BellerFeb 9, 2017
  16. Junio C HamanoFeb 9, 2017
  17. Stefan BellerFeb 9, 2017
  18. 5/5 read_loose_refs(): read refs using resolve_ref_recursively()Michael Haggerty, Feb 9, 2017
  19. Stefan BellerFeb 9, 2017
  20. 2/5 refs: push the submodule attribute downMichael Haggerty, Feb 9, 2017
  21. Stefan BellerFeb 9, 2017
  22. Junio C HamanoFeb 9, 2017
  23. Junio C HamanoFeb 9, 2017
  24. Jeff KingFeb 9, 2017
  25. Michael HaggertyFeb 9, 2017
  26. Jeff KingFeb 10, 2017
  27. Michael HaggertyFeb 10, 2017
  28. Duy NguyenFeb 13, 2017

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.