{"thread":{"id":"55970","subject":"[GSoC] Git Blog 5","startedAt":"2021-06-20T16:53:19Z","lastAt":"2021-06-21T13:18:04Z","messageCount":3,"participants":["ZheNing Hu","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"427989","messageId":"CAOLTT8TZqKQ1vkRX7apvzAJ04Cv9aHJ9kF1+wJcEvyi-pibOOw@mail.gmail.com","threadId":"55970","inReplyTo":null,"subject":"[GSoC] Git Blog 5","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-06-20T16:53:03Z","receivedAt":"2021-06-20T16:53:19Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"My fifth week blog finished:\nThe web version is here:\nhttps://adlternative.github.io/GSOC-Git-Blog-5/\n\n## Week5: Tempting apple\n\nThis week, I spent a lot of time working on the digital circuit\ncourse design of the school. So this week's patches for git\nwas completed in a hurry. This week `Ævar Arnfjörð Bjarmason`\ngave a lot of useful suggestions for the patch I wrote earlier.\nSome are related to code style improvements, and some are\nbetter design ideas.\n\nBefore, I wanted to use a `<s, s_size>` style string in `atom_value`\nto help copy and compare data containing '\\0'.\n\nLike this:\n\n```c\nstruct atom_value {\nconst char *s;\nsize_t s_size;\n...\n};\n```\n\nBut `Ævar Arnfjörð Bjarmason` thinks it is more reasonable to\nuse `strbuf` instead of `<s, s_size>`.\n\nLike this:\n\n```c\nstruct atom_value {\nstruct strbuf s;\n...\n};\n```\n\nSince the `strbuf` API has a natural `<s, s_size>`, we can add data\nthat may contain '\\0' through `strbuf_add()`, `strbuf_addbuf()`, and\n`strbuf_addf()` can also be used to fill strbuf with format string. Use\n `strbuf_addstr()` to replace `xstrdup()`, `strbuf_add()` to replace\n `xmemdupz()`, this is indeed a very tempting choice.\n\nBut in my actual refactoring process, this is not very easy to achieve.\n\nFor example, the original interface is like this,\n\n```c\nv->s = copy_email(wholine, &used_atom[i]);\n```\n\n`v->s` will be filled with the data dynamically allocated by `copy_email()`.\n\n```c\nstatic const char *copy_email(const char *buf, struct used_atom *atom)\n{\n...\nif (!eoemail)\nreturn xstrdup(\"\");\nreturn xmemdupz(email, eoemail - email);\n}\n```\n\nThen if we want use `strbuf` type `v->s`, we should change the\n `copy_email()` interface parameters and return value.\n\n```c\nstatic void copy_email(struct strbuf *str, const char *buf, struct\nused_atom *atom)\n{\n...\nif (!eoemail)\nreturn;\nreturn strbuf_add(str, email, eoemail - email);\n}\n```\n\nThen the caller can do:\n\n```c\ncopy_email(&v->s, wholine, &used_atom[i]);\n```\n\nThis is in line with our expectations.\n\nBut something like `fill_remote_ref_details()`, things gradually\nbecome complicated and difficult. `fill_remote_ref_details()`.\nJust consider the `show_ref()` called in `fill_remote_ref_details()`,\n`show_ref()` may call `shorten_unambiguous_ref()` internally, and\nthen another function is called internally in `shorten_unambiguous_ref()`...\nThis makes us fail the method of passing `v->s` in parameters like\n`copy_email()` does. Another way of thinking: what if I can \"attach\"\nthe data directly? Using `strbuf_attach()` may be a viable option, but...\n\n```c\nsize_t len;\nvoid *data = shorten_unambiguous_ref(refname, warn_ambiguous_refs);\nlen = strlen(data);\nstrbuf_attach(&str, data, len, len);\n```\n\n...we need to get the length of the data, but this is not easy to do,\n`strlen()` can only be used on data that does not contain '\\0',\nand we are not sure whether a function like `shorten_unambiguous_ref()`\nwill return a `NULL`.\n\nWell, this is one of the reasons why I cannot move on.\n\nOn the other hand, look at the following piece of code, it appears\nin `populate_value()`.\n\n```c\nfor (i = 0; i < used_atom_cnt; i++) {\nstruct atom_value *v = &ref->value[i];\nif (v->s == NULL && used_atom[i].source == SOURCE_NONE)\nreturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\noid_to_hex(&ref->objectname), ref->refname);\n}\n```\n\nWe need to determine whether `v->s` equals to `NULL`. We can use\nc-style strings to easily distinguish between empty strings(`xstrdup(\"\")`)\nand `NULL`, but if we use strbuf, it is not easy to distinguish, because\nan empty strbuf has the following characteristics: `s.buf == strbuf_slopbuf`\nor `sb->buf[0] == '\\0'`. It can be said that we shouldn’t even use `NULL`\nto assign to `s.buf`.\n\nSo in the end, I rejected this seemingly very attractive solution, and use the\nprevious strategy: `<s, s_size>`, unless someone can think of a better\nsolution later `;-)`.\n\n* FootNote:\n1. Why is there no `strbuf_empty()` in `strbuf` API? I think this may be\na very important thing.\n\n```c\n#define strbuf_empty(sb) \\\n(sb->buf == strbuf_slopbuf) ? \\\n(!strbuf_slopbuf[0]) : \\\n(sb->buf[0] == '\\0')\n```\n\n2. Another thing worth mentioning is: I will have the school final exam\nbetween July 7th and July 14th, I may be busy during this period.\n\nThanks for Git community, reviewers and mentors.\n--\nZheNing Hu\n"},{"id":"428030","messageId":"CAP8UFD0sHevUHB-ryUZm9Sxm=daKMwbqe6_Q7kc-pZAZN-eHWA@mail.gmail.com","threadId":"55970","inReplyTo":"CAOLTT8TZqKQ1vkRX7apvzAJ04Cv9aHJ9kF1+wJcEvyi-pibOOw@mail.gmail.com","subject":"Re: [GSoC] Git Blog 5","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-06-21T12:07:32Z","receivedAt":"2021-06-21T12:07:46Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Jun 20, 2021 at 6:53 PM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> My fifth week blog finished:\n> The web version is here:\n> https://adlternative.github.io/GSOC-Git-Blog-5/\n\nGreat, thanks!\n\n> ## Week5: Tempting apple\n>\n> This week, I spent a lot of time working on the digital circuit\n> course design of the school. So this week's patches for git\n> was completed in a hurry. This week `Ævar Arnfjörð Bjarmason`\n\ns/was/were/\n\n> gave a lot of useful suggestions for the patch I wrote earlier.\n\nMaybe a link could help.\n\n> Some are related to code style improvements, and some are\n> better design ideas.\n\n[...]\n\n> But something like `fill_remote_ref_details()`, things gradually\n> become complicated and difficult. `fill_remote_ref_details()`.\n\nI think you can remove the above `fill_remote_ref_details()` as you\nare talking about it later.\n\n> Just consider the `show_ref()` called in `fill_remote_ref_details()`,\n> `show_ref()` may call `shorten_unambiguous_ref()` internally, and\n> then another function is called internally in `shorten_unambiguous_ref()`...\n\n> Well, this is one of the reasons why I cannot move on.\n>\n> On the other hand, look at the following piece of code, it appears\n> in `populate_value()`.\n>\n> ```c\n> for (i = 0; i < used_atom_cnt; i++) {\n> struct atom_value *v = &ref->value[i];\n> if (v->s == NULL && used_atom[i].source == SOURCE_NONE)\n> return strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n> oid_to_hex(&ref->objectname), ref->refname);\n> }\n> ```\n>\n> We need to determine whether `v->s` equals to `NULL`. We can use\n\ns/equals to/equals/\n\n> c-style strings to easily distinguish between empty strings(`xstrdup(\"\")`)\n> and `NULL`, but if we use strbuf, it is not easy to distinguish, because\n> an empty strbuf has the following characteristics: `s.buf == strbuf_slopbuf`\n> or `sb->buf[0] == '\\0'`. It can be said that we shouldn’t even use `NULL`\n> to assign to `s.buf`.\n>\n> So in the end, I rejected this seemingly very attractive solution, and use the\n> previous strategy: `<s, s_size>`, unless someone can think of a better\n> solution later `;-)`.\n>\n> * FootNote:\n> 1. Why is there no `strbuf_empty()` in `strbuf` API? I think this may be\n> a very important thing.\n\nThere is a 'size_t len' field in 'struct strbuf', so doing `buf->len\n== 0` works.\n\n> 2. Another thing worth mentioning is: I will have the school final exam\n> between July 7th and July 14th, I may be busy during this period.\n\nOk, thanks for telling us in advance.\n"},{"id":"428034","messageId":"CAOLTT8TYn0x_wuDz7Nn1sKn0VKMyF320p1urxG1KHc+se4aiNA@mail.gmail.com","threadId":"55970","inReplyTo":"CAP8UFD0sHevUHB-ryUZm9Sxm=daKMwbqe6_Q7kc-pZAZN-eHWA@mail.gmail.com","subject":"Re: [GSoC] Git Blog 5","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-06-21T13:17:51Z","receivedAt":"2021-06-21T13:18:04Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Christian Couder <christian.couder@gmail.com> 于2021年6月21日周一 下午8:07写道：\n>\n> On Sun, Jun 20, 2021 at 6:53 PM ZheNing Hu <adlternative@gmail.com> wrote:\n> >\n> > My fifth week blog finished:\n> > The web version is here:\n> > https://adlternative.github.io/GSOC-Git-Blog-5/\n>\n> Great, thanks!\n>\n> > ## Week5: Tempting apple\n> >\n> > This week, I spent a lot of time working on the digital circuit\n> > course design of the school. So this week's patches for git\n> > was completed in a hurry. This week `Ævar Arnfjörð Bjarmason`\n>\n> s/was/were/\n>\n> > gave a lot of useful suggestions for the patch I wrote earlier.\n>\n> Maybe a link could help.\n>\n> > Some are related to code style improvements, and some are\n> > better design ideas.\n>\n> [...]\n>\n> > But something like `fill_remote_ref_details()`, things gradually\n> > become complicated and difficult. `fill_remote_ref_details()`.\n>\n> I think you can remove the above `fill_remote_ref_details()` as you\n> are talking about it later.\n>\n\nOK.\n\n> > * FootNote:\n> > 1. Why is there no `strbuf_empty()` in `strbuf` API? I think this may be\n> > a very important thing.\n>\n> There is a 'size_t len' field in 'struct strbuf', so doing `buf->len\n> == 0` works.\n>\n\nReally? So I think for reader who is learning strbuf api for the first time,\nthe characteristics of empty strbuf are not so obvious. For example,\n`strbuf_slopbuf` may mean \"empty\". In any case, adding a macro\nis absolutely reasonable and will not affect performance.\n\n#define strbuf_empty(sb) (sb->len == 0)\n\n> > 2. Another thing worth mentioning is: I will have the school final exam\n> > between July 7th and July 14th, I may be busy during this period.\n>\n> Ok, thanks for telling us in advance.\n\nCorrect the time deviation, the examination time is from July 1st to July 12th.\n\n--\nZheNing Hu\n"}]}