[PATCH 1/1] enable cgit to show gravatar for author, committer and tagger

Christian Hesse list at eworm.de
Wed Nov 27 23:59:03 CET 2013


Peter Wu <lekensteyn at gmail.com> on Wed, 2013/11/27 23:11:
> On Wednesday 27 November 2013 22:14:22 Christian Hesse wrote:
> > +char * cgit_get_gravatar(const char *email) {
> > +       unsigned char digest[16];
> > +       char hex[33], *lower, *tmp;
> > +       char *gravatar;
> > +
> > +       gravatar = malloc(77);
> 
> I do not quite like this magic number, does anyone have problems with
> creating a GRAVATAR_URL macro? 77 seems also small enough to throw on the
> stack. (Btw, len("//www.gravatar.com/avatar/?s=16&d=retro") + 32 + 1
> says that the length is 76 including nul byte).

Created a GRAVATAR_URL macro now. I do prefer this solution as well.

Got rid of some more magic numbers for MD5 hashing.

> > +       /* duplicate to lower and skip brackets! */
> > +       lower = strdup(email + 1);
> > +       lower[strlen(lower) - 1] = '\0';
> 
> Ahh, I though it was off by one, but apparently you remove the brackets. OK.
> 
> > +
> > +       /* make the chars lower case */
> > +       for (tmp = lower; *tmp; ++tmp)
> > +               *tmp = tolower(*tmp);
> > +
> > +       MD5((unsigned char *)lower, strlen(lower), digest);
> > +
> > +       str_to_hex(hex, digest, 16);
> > +
> > +       sprintf(gravatar,
> > "//www.gravatar.com/avatar/%s?s=16&d=retro", hex);
> 
> It seems ugly to me to store the HTML-escaped URL here. What do you think of
> using just "&" here and html_txt() when outputting the URL? Processing these
> few chars and performing an extra write() should not hurt performance.

Agreed.

> Regarding the GRAVATAR_URL macro mentioned earlier, if used that could
> also be inserted here. A comment could then describe why this URL is used
> (16x16 image using "awesome generated, 8-bit arcade-style pixelated faces"
> as default, https://en.gravatar.com/site/implement/images/)

Done.

> > +       free(lower);
> > +
> > +       return gravatar;
> > +}

I will replay with my updated patch.

-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Chris           get my mail address:    */=0;b=c[a++];)
putchar(b-1/(/*               gcc -o sig sig.c && ./sig    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20131127/49b03159/attachment.asc>


More information about the CGit mailing list