Title: Letter to the Editor

Author: Bob Schmidt

Date: 01 March 2020 21:59:40 +00:00 or Sun, 01 March 2020 21:59:40 +00:00

Summary: Silas S. Brown responds to Ian Bruntlett’s article.

Body: 

Dear Editor,

I enjoyed Ian Bruntlett’s article ‘Static C library and GNU Make’ (C Vu 31.6, January 2020), but I hope he won’t mind my pointing out some improvements that can be made to the code, in the spirit of ACCU being a good learning environment.

Firstly, I’m afraid there is a bug in Listing 2, specifically this part:

  for (int rhs = length-1;
    isspace(text[rhs]);
    --rhs
    )
    text[rhs] = '\0';

Consider what happens when text is a string consisting entirely of spaces and nothing else. In this case the loop decrementing rhs will quite happily set it to -1 and try to read it for isspace(). In most cases, the byte before the string starts will (a) be readable by your program and (b) not happen to contain a space, in which case nothing will happen. But if it does happen to contain a space, then we’re looking at memory corruption or, in the unlikely event that its address is off-limits to your program, you’ll get a segmentation fault.

The easiest fix would be to add a condition for rhs >= 0 in the loop. But if we start using size_t instead of int (which we’d need to do if we want to support strings that are so enormous their lengths cannot be represented by a signed integer on the platform) then we’d have to explicitly write if (rhs==0) break; at the end of the loop body.

Incidentally, it is not really necessary to overwrite every space with \0 when right-trimming; only the first one needs to be overwritten (and this might make a speed difference if memory-writes are slower than memory-reads on the system). So it might be better to write the loop as:

  size_t rhs;
  for (rhs=length; rhs && isspace(text[rhs-1]);
       rhs--);
  text[rhs] = '\0';

and while we’re making such improvements, ltrim() might as well use memmove() rather than writing the loop manually (the library memmove will likely have optimisations that can move multiple bytes at a time using the CPU’s extended registers), and the two ltrimcpy and rtrimcpy functions can be improved to copy only the non-whitespace portion of the string (after all, there’s no point in copying the spaces only to remove them). But on the other hand, programmer time is also a resource and sometimes we shouldn’t use our time optimising things that don’t really need to be optimal; I’m mentioning this only for completeness.

I realise the thrust of the article is about Makefiles and not really about the code itself, but still thought the above might be worth pointing out for the sake of the learners.

Many thanks.

Silas

Silas S. Brown is a partially-sighted Computer Science post-doc in Cambridge who currently works in part-time assistant tuition and part-time for Oracle. He has been an ACCU member since 1994.

Notes: 

More fields may be available via dynamicdata ..