Skip to content

Commit

Permalink
Fixup; fix another warning.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dkager
dkager committed Jan 7, 2016
1 parent f222c77 commit 20545a2
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
3 changes: 2 additions & 1 deletion liblouis/compileTranslationTable.c
View file
Expand Up @@ -5434,8 +5434,9 @@ resolveSubtable (const char *table, const char *base, const char *searchPath)
//
if (base)
{
int k = (int)strlen (tableFile);
int k;
strcpy (tableFile, base);
k = (int)strlen (tableFile);
while (k >= 0 && tableFile[k] != DIR_SEP) k--;
tableFile[++k] = '\0';
strcat (tableFile, table);
Expand Down
2 changes: 1 addition & 1 deletion liblouis/findTable.c
View file
Expand Up @@ -618,7 +618,7 @@ lou_findTable(const char * query)
logMessage(LOG_WARN, "Tables have not been indexed yet. Indexing LOUIS_TABLEPATH.");
searchPath = getTablePath();
tables = listFiles(searchPath);
tablesArray = list_toArray(tables, NULL);
tablesArray = (const char **)list_toArray(tables, NULL);
lou_indexTables(tablesArray);
free(searchPath);
list_free(tables);
Expand Down

5 comments on commit 20545a2

@egli
Copy link
Member

@egli egli commented on 20545a2 Jan 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I don't like these kinds of fixes. IMHO if there is a need for a cast there is something wrong. Why does the assignment of tablesArray have to be cast? I'll have to look at the code. But casting feels like duct taping to me, just making a possibly real problem disappear out of sight.

@bertfrees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Davy just added an explicit cast where I assume on *nix an implicit cast happens. It is needed because List and the list_* functions are generic. Don't know, maybe this is bad coding style. I've seen it done somewhere else, and I don't know how you could otherwise make something generic.

@egli
Copy link
Member

@egli egli commented on 20545a2 Jan 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, maybe I'll have to read up in my copy of 21st Century C

@bertfrees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Don't know if it says anything about it :)

@dkager
Copy link
Contributor Author

@dkager dkager commented on 20545a2 Jan 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, explicitly casting makes clear the intention: use a generic function with an instantiated array of chars. I guess the generics aren't as clean as in an object oriented language, but I have also seen it done before. I also like a clean build log so real problems will catch my attention a bit quicker.

The much more ugly casts, and IMO a real problem are the casts from size_t to int, where int can have different sizes on different platforms (and maybe size_t can too). But we should be 'safe' here since none of these strings should ever reach a length that won't fit in even 16 bits. I considered actually using variables of type size_t, but this cascades down the code and gets all over the place.

Please sign in to comment.