This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] add benchmark for strcoll


On 09 Mar 2015 10:20, Leonhard Holz wrote:
> --- a/benchtests/bench-strcoll.c
> +++ b/benchtests/bench-strcoll.c
>
> +const char *locales[] = {

static.  probably want to do that on all the funcs in this file too.

> +concat_path (const char *dirname, const char *filename)
> +{
> +  size_t d_len = strlen (dirname);
> +  size_t f_len = strlen (filename);
> +  char * path = malloc (d_len + f_len + 2);

no space after the *

> +  memcpy (path, dirname, d_len);
> +  * (path + d_len) = '/';
> +  memcpy (path + d_len + 1, filename, f_len);
> +  * (path + d_len + f_len + 1) = '\0';

this is weird/ugly syntax.  just use a proper bracket:
	path[d_len] = '/';
	path[d_len + f_len + 1] = '\0';

that said, you're simply doing path concatenation here.  replace this whole func 
with a single asprintf:
	char *path;
	int ret = asprintf (&path, "%s/%s", dirname, filename);
	assert (ret > 0);
	return path;

> +char *
> +read_file (const char *filename)
> +{
> +  struct stat stats;
> +  char *buffer = NULL;
> +  int fd = open (filename, O_RDONLY);

i know it doesn't matter here, but my preference is to use O_CLOEXEC everywhere 
unless explicitly not needed.

> +	      read (fd, buffer, stats.st_size);

doesn't this warn about unused results ?

> +	      *(buffer + stats.st_size) = '\0';

please use buffer[stats.st_size] syntax

> +word_list *
> +file_word_list (const char *dirname)
> +{
> +  DIR *dir;
> +  struct dirent *ent;
> +  word_list *list = NULL;
> +
> +  if ((dir = opendir (dirname)) != NULL)
> +    {
> +      size_t ent_cnt = 0, i = 0;
> +      word_list *sublist = new_word_list (0);
> +
> +      while ((ent = readdir (dir)) != NULL)
> +	if (strcmp (".", ent->d_name) != 0 && strcmp ("..", ent->d_name) != 0)

would probably be simpler to just skip all paths that start with a '.':
	if (ent->d_name[0] != '.')

> +  timing_t res __attribute__ ((unused));

use __attribute_used__ instead
-mike

Attachment: signature.asc
Description: Digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]