[ft-devel] problems with your branch

[Werner LEMBERG]

Kushal,


I've just tried the current git version of your branch, and I've found
some problems.

1. In `make_sprite.c', you are allocating `output_file_name' with a
   static size of 100 characters.  However, you are using `sprintf' to
   fill this buffer.  THIS IS A CAPITAL SIN!  At least you should use
   `snprintf', but a better solution is of course to make allocation
   dynamic, not posing any restrictions on the length of the file
   name.

2. You are calling `fopen' without checking for success, another
   capital sin.

3. The `README' file isn't up to date, which is *very* bad.  I
   incorrectly assumed that a call

     FT_TEST_BASE_DIR=/home/wl/kushal/base \
     FT_TEST_TEST_DIR=/home/wl/kushal/test \
     FT_TEST_DPI="72" \
     FT_TEST_FONT_FILE="test.ttf" \
     FT_TEST_RENDER_MODE="AA" \
     FT_TEST_PT_SIZE="16 20" \
       ./runme.sh

   would succeed.  However, `runme.sh' doesn't properly translate `AA'
   to value `1', as expected currently by `make_sprite', and I got
   crashes (due to item 2).  Of course, it would be better if
   `make_sprite' understands `AA' and the other friendly render mode
   strings.

   I had to call the debugger to find this out...

4. You are hard-coding `FT_TEST_TEST_DIR' in `runme.sh', which is bad,
   since I can't overwrite it as an environment variable.  Actually,
   it would be better to not provide XXX_BASE_DIR but XXX_BASE_DLL,
   since the DLLs might be located somewhere else (for example, in
   `/usr/lib64').

5. In `runme.sh', you don't provide default values for missing
   environment variables.  For example, I would like to just say

     FT_TEST_BASE_DIR=/home/wl/kushal/base \
     FT_TEST_TEST_DIR=/home/wl/kushal/test \
     FT_TEST_FONT_FILE="test.ttf" \
       ./runme.sh

   and the script fills up the rest.

   Look at

     
https://stackoverflow.com/questions/2013547/assigning-default-values-to-shell-variables-with-a-single-command-in-bash

   how to do that.

6. In the Makefile, you are still linking to libfreetype.a, which is
   completely unnecessary.  Attached is a patch.

7. Using render mode value `2' crashed `make_sprite' if compiled with
   optimization.  However, I couldn't repeat that using CFLAGS="-O0
   -g3" during configuration, so maybe I did something wrong or just
   saw a Heisenbug...

8. Using valgrind, I see huge memory leaks for `make_sprite.c'...
   I've attached a log file.

9. If I load `top.html' the first time (or reload it with key F5), no
   glyphs are displayed.  I have to explicitly change the size to see
   something.  This happens on Chrome 58.

10. Looking at the HTML output for sizes 16 and 20 of `test.ttf'
    (using 72dpi and rendering mode 2), then switching between sizes
    16 and 20, I see that the scaling of the glyph doesn't fit – the
    used glyph scaling values should be integer multiples and nothing
    else...


    Werner

valgrind.log.xz
Description: Binary data