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

Re: [PATCH v3 03/12] object-file API: add a format_object_header() function

From
Jiang Xin <worldhello.net@gmail.com>
Date
Mar 1, 2022, 03:09 UTC
Message-ID
<CANYiYbFZN=mvcxRT4Cf4Gzao89KR2UjMJXnJM63S2zAtbD32Jg@mail.gmail.com>
In-Reply-To
<220217.86mtipj14m.gmgdl@evledraar.gmail.com>

On Thu, Feb 17, 2022 at 5:25 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 48 quoted lines
>
>
> On Thu, Feb 17 2022, Jiang Xin wrote:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > Type casting can be avoid if we use "void *" as the first parameter of
> > "format_object_header", and do type casting inside the helper function.
> > [...]
> > The return value of `type_name(type)` has not been checked for the original
> > implement, how about write a online inline-function in a header file like this:
> >
> >       static inline int format_object_header(void *str, size_t size,
> >                                              const char *type_name,
> >                                              size_t objsize)
> >       {
> >               return xsnprintf((char *)str, size, "%s %"PRIuMAX, type_name,
> >                                (uintmax_t)objsize) + 1;
> >       }
>
> I don't think the casting in the callers is bad, in that for the callers
> that do use "char *" we get the compiler to help us with type checking.
>
> Using a void * is something we really reserve only for callback-type
> values, because it mens that now nobody gets any type checking.
>
> I think if we wanted to avoid the casts it would make more sense to add
> a trivial ucformat_object_header() wrapper or whatever, which would take
> "unsigned chan *" and do the cast, or just tweak the relevant calling
> code to change the type (IIRC some of it used unsigned v.s. signed for
> no particular reason).
>
> But I think just leaving this part as it is is better here...
>
> > [...]
> >> +    if (!name)
> >> +            BUG("could not get a type name for 'enum object_type' value %d", type);
> >> +
> >
> > The return value of `type_name(type)` has not been checked for the original
> > implement, how about write a online inline-function in a header file like this:
>
> Yes, this part is not a faithful conversion on my part, but I think it
> made sense when converting this to a library function.
>
> The alternative is that we'd segfault on some platforms (not glibc,
> since it's OK with null %s arguments), just checking it is cheap & I
> think a good sanity check...

After sending the last email, I was offline for over a week due to an emergency at home.

According to the rest patches of this series, your implementation of "format_object_header()" which uses "enum object_type" as the parameter instead of "char *type" is better. And adding a validation for `type_name(type)` is also better than my one-line inline function. This series of patches LGTM.

-- Jiang Xin

Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 37 of 46 in “object-file API: pass object enums, tidy up streaming interface”
  1. 00/10 object-file API: pass object enums, tidy up streaming interfaceÆvar Arnfjörð Bjarmason, Feb 1, 2022
  2. 01/10 object-file.c: split up declaration of unrelated variablesÆvar Arnfjörð Bjarmason, Feb 1, 2022
  3. 02/10 object-file API: return "void", not "int" from hash_object_file()Ævar Arnfjörð Bjarmason, Feb 1, 2022
  4. 03/10 object-file API: add a format_object_header() functionÆvar Arnfjörð Bjarmason, Feb 1, 2022
  5. 04/10 object-file API: have write_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 1, 2022
  6. Junio C HamanoFeb 1, 2022
  7. 05/10 object-file API: provide a hash_object_file_oideq()Ævar Arnfjörð Bjarmason, Feb 1, 2022
  8. Junio C HamanoFeb 1, 2022
  9. Ævar Arnfjörð BjarmasonFeb 1, 2022
  10. 06/10 object-file API: replace some use of check_object_signature()Ævar Arnfjörð Bjarmason, Feb 1, 2022
  11. Junio C HamanoFeb 1, 2022
  12. 08/10 object-file API: replace check_object_signature() with stream_*Ævar Arnfjörð Bjarmason, Feb 1, 2022
  13. 07/10 object-file API: have hash_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 1, 2022
  14. 09/10 object-file.c: add a literal version of write_object_file_prepare()Ævar Arnfjörð Bjarmason, Feb 1, 2022
  15. 10/10 object-file API: pass an enum to read_object_with_reference()Ævar Arnfjörð Bjarmason, Feb 1, 2022
  16. 00/11 object-file API: pass object enums, tidy up streaming interfaceÆvar Arnfjörð Bjarmason, Feb 4, 2022
  17. 02/11 object-file API: return "void", not "int" from hash_object_file()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  18. 03/11 object-file API: add a format_object_header() functionÆvar Arnfjörð Bjarmason, Feb 4, 2022
  19. 04/11 object-file API: have write_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 4, 2022
  20. Junio C HamanoFeb 4, 2022
  21. 01/11 object-file.c: split up declaration of unrelated variablesÆvar Arnfjörð Bjarmason, Feb 4, 2022
  22. 05/11 object API: correct "buf" v.s. "map" mismatch in *.c and *.hÆvar Arnfjörð Bjarmason, Feb 4, 2022
  23. Junio C HamanoFeb 4, 2022
  24. 06/11 object API: make check_object_signature() oideq()-like, move docsÆvar Arnfjörð Bjarmason, Feb 4, 2022
  25. Junio C HamanoFeb 4, 2022
  26. 07/11 object-file API: split up and simplify check_object_signature()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  27. 08/11 object API: rename hash_object_file_literally() to write_*()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  28. 09/11 object-file API: have hash_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 4, 2022
  29. 10/11 object-file.c: add a literal version of write_object_file_prepare()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  30. 11/11 object-file API: pass an enum to read_object_with_reference()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  31. 00/12 object-file API: pass object enums, tidy up streaming interfaceÆvar Arnfjörð Bjarmason, Feb 4, 2022
  32. 01/12 object-file.c: split up declaration of unrelated variablesÆvar Arnfjörð Bjarmason, Feb 4, 2022
  33. 02/12 object-file API: return "void", not "int" from hash_object_file()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  34. 03/12 object-file API: add a format_object_header() functionÆvar Arnfjörð Bjarmason, Feb 4, 2022
  35. Jiang XinFeb 17, 2022
  36. Ævar Arnfjörð BjarmasonFeb 17, 2022
  37. Jiang XinMar 1, 2022
  38. 04/12 object-file API: have write_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 4, 2022
  39. 05/12 object API: correct "buf" v.s. "map" mismatch in *.c and *.hÆvar Arnfjörð Bjarmason, Feb 4, 2022
  40. 06/12 object API docs: move check_object_signature() docs to cache.hÆvar Arnfjörð Bjarmason, Feb 4, 2022
  41. 09/12 object API: rename hash_object_file_literally() to write_*()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  42. 08/12 object-file API: split up and simplify check_object_signature()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  43. 07/12 object API users + docs: check <0, not !0 with check_object_signature()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  44. 10/12 object-file API: have hash_object_file() take "enum object_type"Ævar Arnfjörð Bjarmason, Feb 4, 2022
  45. 12/12 object-file API: pass an enum to read_object_with_reference()Ævar Arnfjörð Bjarmason, Feb 4, 2022
  46. 11/12 object-file.c: add a literal version of write_object_file_prepare()Ævar Arnfjörð Bjarmason, Feb 4, 2022

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.