* [PATCH 1 of 2] libxlu: Rename filename to config_source [not found] <patchbomb.1333477720@kodo2> @ 2012-04-03 18:28 ` George Dunlap 2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap 1 sibling, 0 replies; 6+ messages in thread From: George Dunlap @ 2012-04-03 18:28 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap The "filename" is a bit of a misnomer, as it's only used during error messages, and in most instances cases is actually set to "command line". Rename it to "config_source" to make it clear that it's not used to actually open any files. No functional changes. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> diff -r 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_cfg.c --- a/tools/libxl/libxlu_cfg.c Tue Apr 03 14:46:27 2012 +0100 +++ b/tools/libxl/libxlu_cfg.c Tue Apr 03 18:00:24 2012 +0100 @@ -25,15 +25,15 @@ #include "libxlu_cfg_l.h" #include "libxlu_cfg_i.h" -XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename) { +XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) { XLU_Config *cfg; cfg= malloc(sizeof(*cfg)); if (!cfg) return 0; cfg->report= report; - cfg->filename= strdup(report_filename); - if (!cfg->filename) { free(cfg); return 0; } + cfg->config_source= strdup(report_source); + if (!cfg->config_source) { free(cfg); return 0; } cfg->settings= 0; return cfg; @@ -51,7 +51,7 @@ static int ctx_prep(CfgParseContext *ctx e= xlu__cfg_yylex_init_extra(ctx, &ctx->scanner); if (e) { fprintf(cfg->report,"%s: unable to create scanner: %s\n", - cfg->filename, strerror(e)); + cfg->config_source, strerror(e)); return e; } return 0; @@ -117,7 +117,7 @@ int xlu_cfg_readdata(XLU_Config *cfg, co buf = xlu__cfg_yy_scan_bytes(data, length, ctx.scanner); if (!buf) { fprintf(cfg->report,"%s: unable to allocate scanner buffer\n", - cfg->filename); + cfg->config_source); ctx.err= ENOMEM; goto xe; } @@ -151,7 +151,7 @@ void xlu_cfg_destroy(XLU_Config *cfg) { set_next= set->next; xlu__cfg_set_free(set); } - free(cfg->filename); + free(cfg->config_source); free(cfg); } @@ -178,7 +178,7 @@ static int find_atom(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is" " a list but should be a single value\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *set_r= set; @@ -223,14 +223,14 @@ int xlu_cfg_get_long(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' could not be parsed" " as a number: %s\n", - cfg->filename, set->lineno, n, strerror(e)); + cfg->config_source, set->lineno, n, strerror(e)); return e; } if (*ep || ep==set->values[0]) { if (!dont_warn) fprintf(cfg->report, "%s:%d: warning: parameter `%s' is not a valid number\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *value_r= l; @@ -258,7 +258,7 @@ int xlu_cfg_get_list(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is a single value" " but should be a list\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); } return EINVAL; } @@ -467,7 +467,7 @@ void xlu__cfg_yyerror(YYLTYPE *loc, CfgP fprintf(ctx->cfg->report, "%s:%d: config parsing error near %s%.*s%s%s: %s\n", - ctx->cfg->filename, lineno, + ctx->cfg->config_source, lineno, len?"`":"", len, text, len?"'":"", newline, msg); if (!ctx->err) ctx->err= EINVAL; diff -r 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_disk.c --- a/tools/libxl/libxlu_disk.c Tue Apr 03 14:46:27 2012 +0100 +++ b/tools/libxl/libxlu_disk.c Tue Apr 03 18:00:24 2012 +0100 @@ -10,7 +10,7 @@ void xlu__disk_err(DiskParseContext *dpc "%s: config parsing error in disk specification: %s" "%s%s%s" " in `%s'\n", - dpc->cfg->filename, message, + dpc->cfg->config_source, message, erroneous?": near `":"", erroneous?erroneous:"", erroneous?"'":"", dpc->spec); if (!dpc->err) dpc->err= EINVAL; diff -r 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_internal.h --- a/tools/libxl/libxlu_internal.h Tue Apr 03 14:46:27 2012 +0100 +++ b/tools/libxl/libxlu_internal.h Tue Apr 03 18:00:24 2012 +0100 @@ -36,7 +36,7 @@ struct XLU_ConfigSetting { /* transparen struct XLU_Config { XLU_ConfigSetting *settings; FILE *report; - char *filename; + char *config_source; }; typedef struct { ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" [not found] <patchbomb.1333477720@kodo2> 2012-04-03 18:28 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap @ 2012-04-03 18:28 ` George Dunlap 2012-04-03 18:52 ` George Dunlap 2012-04-04 15:18 ` Ian Jackson 1 sibling, 2 replies; 6+ messages in thread From: George Dunlap @ 2012-04-03 18:28 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap Many places in xl there's a variable or element named "filename" which does not contain a filename, but the source of the data for reporting purposes. Worse, there are variables which are sometimes actually used or interpreted as a filename depending on what other variables are set. This makes it difficult to tell when a string is purely cosmetic, and when another bit of code may actually attempt to call "open" with the string. This patch makes a consistent distinction between "filename" (which always refers to the name of an actual file, and may be interpreted as such at some point) and "source" (which may be a filename, or may be another data source such as a migration stream or saved data). This does add some variables and reshuffle where assignments happen; most notably, the "restore_filename" element of struct domain_create is now only set when restoring from a file. But at a high level, there should be no funcitonal changes. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c Tue Apr 03 18:00:24 2012 +0100 +++ b/tools/libxl/libxl_utils.c Tue Apr 03 19:02:19 2012 +0100 @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx * \ int libxl_##rw##_exactly(libxl_ctx *ctx, int fd, \ constdata void *data, ssize_t sz, \ - const char *filename, const char *what) { \ + const char *source, const char *what) { \ ssize_t got; \ \ while (sz > 0) { \ @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx * if (errno == EINTR) continue; \ if (!ctx) return errno; \ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \ - what?what:"", what?" from ":"", filename); \ + what?what:"", what?" from ":"", source); \ return errno; \ } \ if (got == 0) { \ @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx * zero_is_eof \ ? "file/stream truncated reading %s%s%s" \ : "file/stream write returned 0! writing %s%s%s", \ - what?what:"", what?" from ":"", filename); \ + what?what:"", what?" from ":"", source); \ return EPROTO; \ } \ sz -= got; \ diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Tue Apr 03 18:00:24 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Tue Apr 03 19:02:19 2012 +0100 @@ -507,9 +507,9 @@ vcpp_out: return rc; } -static void parse_config_data(const char *configfile_filename_report, - const char *configfile_data, - int configfile_len, +static void parse_config_data(const char *config_source, + const char *config_data, + int config_len, libxl_domain_config *d_config) { const char *buf; @@ -524,15 +524,15 @@ static void parse_config_data(const char libxl_domain_create_info *c_info = &d_config->c_info; libxl_domain_build_info *b_info = &d_config->b_info; - config= xlu_cfg_init(stderr, configfile_filename_report); + config= xlu_cfg_init(stderr, config_source); if (!config) { fprintf(stderr, "Failed to allocate for configuration\n"); exit(1); } - e= xlu_cfg_readdata(config, configfile_data, configfile_len); + e= xlu_cfg_readdata(config, config_data, config_len); if (e) { - fprintf(stderr, "Failed to parse config file: %s\n", strerror(e)); + fprintf(stderr, "Failed to parse config: %s\n", strerror(e)); exit(1); } @@ -1467,6 +1467,8 @@ static int create_domain(struct domain_c const char *config_file = dom_info->config_file; const char *extra_config = dom_info->extra_config; const char *restore_file = dom_info->restore_file; + const char *config_source = NULL; + const char *restore_source = NULL; int migrate_fd = dom_info->migrate_fd; int i; @@ -1482,24 +1484,28 @@ static int create_domain(struct domain_c pid_t child_console_pid = -1; struct save_file_header hdr; + int restoring = (restore_file || (migrate_fd >= 0)); + memset(&d_config, 0x00, sizeof(d_config)); - if (restore_file) { + if (restoring) { uint8_t *optdata_begin = 0; const uint8_t *optdata_here = 0; union { uint32_t u32; char b[4]; } u32buf; uint32_t badflags; if (migrate_fd >= 0) { + restore_source = "incoming migration stream"; restore_fd = migrate_fd; } else { + restore_source = restore_file; restore_fd = open(restore_file, O_RDONLY); rc = libxl_fd_set_cloexec(ctx, restore_fd, 1); if (rc) return rc; } CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr, - sizeof(hdr), restore_file, "header") ); + sizeof(hdr), restore_source, "header") ); if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) { fprintf(stderr, "File has wrong magic number -" " corrupt or for a different tool?\n"); @@ -1512,7 +1518,7 @@ static int create_domain(struct domain_c fprintf(stderr, "Loading new save file %s" " (new xl fmt info" " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n", - restore_file, hdr.mandatory_flags, hdr.optional_flags, + restore_source, hdr.mandatory_flags, hdr.optional_flags, hdr.optional_data_len); badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ ); @@ -1525,7 +1531,7 @@ static int create_domain(struct domain_c if (hdr.optional_data_len) { optdata_begin = xmalloc(hdr.optional_data_len); CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin, - hdr.optional_data_len, restore_file, "optdata") ); + hdr.optional_data_len, restore_source, "optdata") ); } #define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin)) @@ -1560,7 +1566,7 @@ static int create_domain(struct domain_c &config_data, &config_len); if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n", config_file, strerror(errno)); return ERROR_FAIL; } - if (!restore_file && extra_config && strlen(extra_config)) { + if (!restoring && extra_config && strlen(extra_config)) { if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) { fprintf(stderr, "Failed to attach extra configration\n"); return ERROR_FAIL; @@ -1575,19 +1581,20 @@ static int create_domain(struct domain_c config_len += sprintf(config_data + config_len, "\n%s\n", extra_config); } + config_source=config_file; } else { if (!config_data) { fprintf(stderr, "Config file not specified and" " none in save file\n"); return ERROR_INVAL; } - config_file = "<saved>"; + config_source = "<saved>"; } if (!dom_info->quiet) - printf("Parsing config file %s\n", config_file); - - parse_config_data(config_file, config_data, config_len, &d_config); + printf("Parsing config from %s\n", config_source); + + parse_config_data(config_source, config_data, config_len, &d_config); if (migrate_fd >= 0) { if (d_config.c_info.name) { @@ -1638,7 +1645,7 @@ start: cb = NULL; } - if ( restore_file ) { + if ( restoring ) { ret = libxl_domain_create_restore(ctx, &d_config, cb, &child_console_pid, &domid, restore_fd); @@ -2410,7 +2417,7 @@ static void list_domains_details(const l { libxl_domain_config d_config; - char *config_file; + char *config_source; uint8_t *data; int i, len, rc; @@ -2421,13 +2428,13 @@ static void list_domains_details(const l rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len); if (rc) continue; - CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid)); + CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid)); memset(&d_config, 0x00, sizeof(d_config)); - parse_config_data(config_file, (char *)data, len, &d_config); + parse_config_data(config_source, (char *)data, len, &d_config); printf_info(default_output_format, info[i].domid, &d_config); libxl_domain_config_dispose(&d_config); free(data); - free(config_file); + free(config_source); } } @@ -2530,7 +2537,7 @@ static void save_domain_core_begin(const } } -static void save_domain_core_writeconfig(int fd, const char *filename, +static void save_domain_core_writeconfig(int fd, const char *source, const uint8_t *config_data, int config_len) { struct save_file_header hdr; @@ -2559,13 +2566,13 @@ static void save_domain_core_writeconfig /* that's the optional data */ CHK_ERRNO( libxl_write_exactly(ctx, fd, - &hdr, sizeof(hdr), filename, "header") ); + &hdr, sizeof(hdr), source, "header") ); CHK_ERRNO( libxl_write_exactly(ctx, fd, - optdata_begin, hdr.optional_data_len, filename, "header") ); + optdata_begin, hdr.optional_data_len, source, "header") ); fprintf(stderr, "Saving to %s new xl format (info" " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n", - filename, hdr.mandatory_flags, hdr.optional_flags, + source, hdr.mandatory_flags, hdr.optional_flags, hdr.optional_data_len); } @@ -2891,7 +2898,6 @@ static void migrate_receive(int debug, i dom_info.daemonize = daemonize; dom_info.monitor = monitor; dom_info.paused = 1; - dom_info.restore_file = "incoming migration stream"; dom_info.migrate_fd = 0; /* stdin */ dom_info.migration_domname_r = &migration_domname; dom_info.incr_generationid = 0; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" 2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap @ 2012-04-03 18:52 ` George Dunlap 2012-04-04 15:18 ` Ian Jackson 1 sibling, 0 replies; 6+ messages in thread From: George Dunlap @ 2012-04-03 18:52 UTC (permalink / raw) To: xen-devel@lists.xensource.com On 03/04/12 19:28, George Dunlap wrote: > Many places in xl there's a variable or element named "filename" which > does not contain a filename, but the source of the data for reporting > purposes. Worse, there are variables which are sometimes actually > used or interpreted as a filename depending on what other variables > are set. This makes it difficult to tell when a string is purely > cosmetic, and when another bit of code may actually attempt to call > "open" with the string. > > This patch makes a consistent distinction between "filename" (which > always refers to the name of an actual file, and may be interpreted as > such at some point) and "source" (which may be a filename, or may be > another data source such as a migration stream or saved data). > > This does add some variables and reshuffle where assignments happen; > most notably, the "restore_filename" element of struct domain_create > is now only set when restoring from a file. > > But at a high level, there should be no funcitonal changes. > > Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> Sorry, this was meant to be an RFC patch before I did more thorough testing to make sure there aren't actually any regressions. Please give your opinions but do not apply yet. > > diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/libxl_utils.c > --- a/tools/libxl/libxl_utils.c Tue Apr 03 18:00:24 2012 +0100 > +++ b/tools/libxl/libxl_utils.c Tue Apr 03 19:02:19 2012 +0100 > @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx * > \ > int libxl_##rw##_exactly(libxl_ctx *ctx, int fd, \ > constdata void *data, ssize_t sz, \ > - const char *filename, const char *what) { \ > + const char *source, const char *what) { \ > ssize_t got; \ > \ > while (sz> 0) { \ > @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx * > if (errno == EINTR) continue; \ > if (!ctx) return errno; \ > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \ > - what?what:"", what?" from ":"", filename); \ > + what?what:"", what?" from ":"", source); \ > return errno; \ > } \ > if (got == 0) { \ > @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx * > zero_is_eof \ > ? "file/stream truncated reading %s%s%s" \ > : "file/stream write returned 0! writing %s%s%s", \ > - what?what:"", what?" from ":"", filename); \ > + what?what:"", what?" from ":"", source); \ > return EPROTO; \ > } \ > sz -= got; \ > diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Tue Apr 03 18:00:24 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue Apr 03 19:02:19 2012 +0100 > @@ -507,9 +507,9 @@ vcpp_out: > return rc; > } > > -static void parse_config_data(const char *configfile_filename_report, > - const char *configfile_data, > - int configfile_len, > +static void parse_config_data(const char *config_source, > + const char *config_data, > + int config_len, > libxl_domain_config *d_config) > { > const char *buf; > @@ -524,15 +524,15 @@ static void parse_config_data(const char > libxl_domain_create_info *c_info =&d_config->c_info; > libxl_domain_build_info *b_info =&d_config->b_info; > > - config= xlu_cfg_init(stderr, configfile_filename_report); > + config= xlu_cfg_init(stderr, config_source); > if (!config) { > fprintf(stderr, "Failed to allocate for configuration\n"); > exit(1); > } > > - e= xlu_cfg_readdata(config, configfile_data, configfile_len); > + e= xlu_cfg_readdata(config, config_data, config_len); > if (e) { > - fprintf(stderr, "Failed to parse config file: %s\n", strerror(e)); > + fprintf(stderr, "Failed to parse config: %s\n", strerror(e)); > exit(1); > } > > @@ -1467,6 +1467,8 @@ static int create_domain(struct domain_c > const char *config_file = dom_info->config_file; > const char *extra_config = dom_info->extra_config; > const char *restore_file = dom_info->restore_file; > + const char *config_source = NULL; > + const char *restore_source = NULL; > int migrate_fd = dom_info->migrate_fd; > > int i; > @@ -1482,24 +1484,28 @@ static int create_domain(struct domain_c > pid_t child_console_pid = -1; > struct save_file_header hdr; > > + int restoring = (restore_file || (migrate_fd>= 0)); > + > memset(&d_config, 0x00, sizeof(d_config)); > > - if (restore_file) { > + if (restoring) { > uint8_t *optdata_begin = 0; > const uint8_t *optdata_here = 0; > union { uint32_t u32; char b[4]; } u32buf; > uint32_t badflags; > > if (migrate_fd>= 0) { > + restore_source = "incoming migration stream"; > restore_fd = migrate_fd; > } else { > + restore_source = restore_file; > restore_fd = open(restore_file, O_RDONLY); > rc = libxl_fd_set_cloexec(ctx, restore_fd, 1); > if (rc) return rc; > } > > CHK_ERRNO( libxl_read_exactly(ctx, restore_fd,&hdr, > - sizeof(hdr), restore_file, "header") ); > + sizeof(hdr), restore_source, "header") ); > if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) { > fprintf(stderr, "File has wrong magic number -" > " corrupt or for a different tool?\n"); > @@ -1512,7 +1518,7 @@ static int create_domain(struct domain_c > fprintf(stderr, "Loading new save file %s" > " (new xl fmt info" > " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n", > - restore_file, hdr.mandatory_flags, hdr.optional_flags, > + restore_source, hdr.mandatory_flags, hdr.optional_flags, > hdr.optional_data_len); > > badflags = hdr.mandatory_flags& ~( 0 /* none understood yet */ ); > @@ -1525,7 +1531,7 @@ static int create_domain(struct domain_c > if (hdr.optional_data_len) { > optdata_begin = xmalloc(hdr.optional_data_len); > CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin, > - hdr.optional_data_len, restore_file, "optdata") ); > + hdr.optional_data_len, restore_source, "optdata") ); > } > > #define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin)) > @@ -1560,7 +1566,7 @@ static int create_domain(struct domain_c > &config_data,&config_len); > if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n", > config_file, strerror(errno)); return ERROR_FAIL; } > - if (!restore_file&& extra_config&& strlen(extra_config)) { > + if (!restoring&& extra_config&& strlen(extra_config)) { > if (config_len> INT_MAX - (strlen(extra_config) + 2 + 1)) { > fprintf(stderr, "Failed to attach extra configration\n"); > return ERROR_FAIL; > @@ -1575,19 +1581,20 @@ static int create_domain(struct domain_c > config_len += sprintf(config_data + config_len, "\n%s\n", > extra_config); > } > + config_source=config_file; > } else { > if (!config_data) { > fprintf(stderr, "Config file not specified and" > " none in save file\n"); > return ERROR_INVAL; > } > - config_file = "<saved>"; > + config_source = "<saved>"; > } > > if (!dom_info->quiet) > - printf("Parsing config file %s\n", config_file); > - > - parse_config_data(config_file, config_data, config_len,&d_config); > + printf("Parsing config from %s\n", config_source); > + > + parse_config_data(config_source, config_data, config_len,&d_config); > > if (migrate_fd>= 0) { > if (d_config.c_info.name) { > @@ -1638,7 +1645,7 @@ start: > cb = NULL; > } > > - if ( restore_file ) { > + if ( restoring ) { > ret = libxl_domain_create_restore(ctx,&d_config, > cb,&child_console_pid, > &domid, restore_fd); > @@ -2410,7 +2417,7 @@ static void list_domains_details(const l > { > libxl_domain_config d_config; > > - char *config_file; > + char *config_source; > uint8_t *data; > int i, len, rc; > > @@ -2421,13 +2428,13 @@ static void list_domains_details(const l > rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl",&data,&len); > if (rc) > continue; > - CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid)); > + CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid)); > memset(&d_config, 0x00, sizeof(d_config)); > - parse_config_data(config_file, (char *)data, len,&d_config); > + parse_config_data(config_source, (char *)data, len,&d_config); > printf_info(default_output_format, info[i].domid,&d_config); > libxl_domain_config_dispose(&d_config); > free(data); > - free(config_file); > + free(config_source); > } > } > > @@ -2530,7 +2537,7 @@ static void save_domain_core_begin(const > } > } > > -static void save_domain_core_writeconfig(int fd, const char *filename, > +static void save_domain_core_writeconfig(int fd, const char *source, > const uint8_t *config_data, int config_len) > { > struct save_file_header hdr; > @@ -2559,13 +2566,13 @@ static void save_domain_core_writeconfig > /* that's the optional data */ > > CHK_ERRNO( libxl_write_exactly(ctx, fd, > -&hdr, sizeof(hdr), filename, "header") ); > +&hdr, sizeof(hdr), source, "header") ); > CHK_ERRNO( libxl_write_exactly(ctx, fd, > - optdata_begin, hdr.optional_data_len, filename, "header") ); > + optdata_begin, hdr.optional_data_len, source, "header") ); > > fprintf(stderr, "Saving to %s new xl format (info" > " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n", > - filename, hdr.mandatory_flags, hdr.optional_flags, > + source, hdr.mandatory_flags, hdr.optional_flags, > hdr.optional_data_len); > } > > @@ -2891,7 +2898,6 @@ static void migrate_receive(int debug, i > dom_info.daemonize = daemonize; > dom_info.monitor = monitor; > dom_info.paused = 1; > - dom_info.restore_file = "incoming migration stream"; > dom_info.migrate_fd = 0; /* stdin */ > dom_info.migration_domname_r =&migration_domname; > dom_info.incr_generationid = 0; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" 2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap 2012-04-03 18:52 ` George Dunlap @ 2012-04-04 15:18 ` Ian Jackson 1 sibling, 0 replies; 6+ messages in thread From: Ian Jackson @ 2012-04-04 15:18 UTC (permalink / raw) To: George Dunlap; +Cc: xen-devel George Dunlap writes ("[Xen-devel] [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source""): > Many places in xl there's a variable or element named "filename" which > does not contain a filename, but the source of the data for reporting > purposes. Worse, there are variables which are sometimes actually > used or interpreted as a filename depending on what other variables > are set. This makes it difficult to tell when a string is purely > cosmetic, and when another bit of code may actually attempt to call > "open" with the string. > > This patch makes a consistent distinction between "filename" (which > always refers to the name of an actual file, and may be interpreted as > such at some point) and "source" (which may be a filename, or may be > another data source such as a migration stream or saved data). I think this is a worthy goal. Ian. ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <patchbomb.1335971421@kodo2>]
* [PATCH 1 of 2] libxlu: Rename filename to config_source [not found] <patchbomb.1335971421@kodo2> @ 2012-05-02 15:10 ` George Dunlap 0 siblings, 0 replies; 6+ messages in thread From: George Dunlap @ 2012-05-02 15:10 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap # HG changeset patch # User George Dunlap <george.dunlap@eu.citrix.com> # Date 1335971126 -3600 # Node ID c3eda4333f1562d68c8b56ae3ef9d73e6b68d873 # Parent c9f97681552fd9aaf3157f30e1f502d9db17f395 libxlu: Rename filename to config_source The "filename" is a bit of a misnomer, as it's only used during error messages, and in most instances cases is actually set to "command line". Rename it to "config_source" to make it clear that it's not used to actually open any files. No functional changes. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> diff -r c9f97681552f -r c3eda4333f15 tools/libxl/libxlu_cfg.c --- a/tools/libxl/libxlu_cfg.c Wed May 02 14:52:55 2012 +0100 +++ b/tools/libxl/libxlu_cfg.c Wed May 02 16:05:26 2012 +0100 @@ -25,15 +25,15 @@ #include "libxlu_cfg_l.h" #include "libxlu_cfg_i.h" -XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename) { +XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) { XLU_Config *cfg; cfg= malloc(sizeof(*cfg)); if (!cfg) return 0; cfg->report= report; - cfg->filename= strdup(report_filename); - if (!cfg->filename) { free(cfg); return 0; } + cfg->config_source= strdup(report_source); + if (!cfg->config_source) { free(cfg); return 0; } cfg->settings= 0; return cfg; @@ -51,7 +51,7 @@ static int ctx_prep(CfgParseContext *ctx e= xlu__cfg_yylex_init_extra(ctx, &ctx->scanner); if (e) { fprintf(cfg->report,"%s: unable to create scanner: %s\n", - cfg->filename, strerror(e)); + cfg->config_source, strerror(e)); return e; } return 0; @@ -117,7 +117,7 @@ int xlu_cfg_readdata(XLU_Config *cfg, co buf = xlu__cfg_yy_scan_bytes(data, length, ctx.scanner); if (!buf) { fprintf(cfg->report,"%s: unable to allocate scanner buffer\n", - cfg->filename); + cfg->config_source); ctx.err= ENOMEM; goto xe; } @@ -151,7 +151,7 @@ void xlu_cfg_destroy(XLU_Config *cfg) { set_next= set->next; xlu__cfg_set_free(set); } - free(cfg->filename); + free(cfg->config_source); free(cfg); } @@ -178,7 +178,7 @@ static int find_atom(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is" " a list but should be a single value\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *set_r= set; @@ -223,14 +223,14 @@ int xlu_cfg_get_long(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' could not be parsed" " as a number: %s\n", - cfg->filename, set->lineno, n, strerror(e)); + cfg->config_source, set->lineno, n, strerror(e)); return e; } if (*ep || ep==set->values[0]) { if (!dont_warn) fprintf(cfg->report, "%s:%d: warning: parameter `%s' is not a valid number\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *value_r= l; @@ -258,7 +258,7 @@ int xlu_cfg_get_list(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is a single value" " but should be a list\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); } return EINVAL; } @@ -467,7 +467,7 @@ void xlu__cfg_yyerror(YYLTYPE *loc, CfgP fprintf(ctx->cfg->report, "%s:%d: config parsing error near %s%.*s%s%s: %s\n", - ctx->cfg->filename, lineno, + ctx->cfg->config_source, lineno, len?"`":"", len, text, len?"'":"", newline, msg); if (!ctx->err) ctx->err= EINVAL; diff -r c9f97681552f -r c3eda4333f15 tools/libxl/libxlu_disk.c --- a/tools/libxl/libxlu_disk.c Wed May 02 14:52:55 2012 +0100 +++ b/tools/libxl/libxlu_disk.c Wed May 02 16:05:26 2012 +0100 @@ -10,7 +10,7 @@ void xlu__disk_err(DiskParseContext *dpc "%s: config parsing error in disk specification: %s" "%s%s%s" " in `%s'\n", - dpc->cfg->filename, message, + dpc->cfg->config_source, message, erroneous?": near `":"", erroneous?erroneous:"", erroneous?"'":"", dpc->spec); if (!dpc->err) dpc->err= EINVAL; diff -r c9f97681552f -r c3eda4333f15 tools/libxl/libxlu_internal.h --- a/tools/libxl/libxlu_internal.h Wed May 02 14:52:55 2012 +0100 +++ b/tools/libxl/libxlu_internal.h Wed May 02 16:05:26 2012 +0100 @@ -38,7 +38,7 @@ struct XLU_ConfigSetting { /* transparen struct XLU_Config { XLU_ConfigSetting *settings; FILE *report; - char *filename; + char *config_source; }; typedef struct { ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0 of 2] libxl cleanup: distinguish filenames from sources @ 2012-05-15 14:51 George Dunlap 2012-05-15 14:51 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap 0 siblings, 1 reply; 6+ messages in thread From: George Dunlap @ 2012-05-15 14:51 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap libxl cleanup: distinguish filenames from sources ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1 of 2] libxlu: Rename filename to config_source 2012-05-15 14:51 [PATCH 0 of 2] libxl cleanup: distinguish filenames from sources George Dunlap @ 2012-05-15 14:51 ` George Dunlap 0 siblings, 0 replies; 6+ messages in thread From: George Dunlap @ 2012-05-15 14:51 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap The "filename" is a bit of a misnomer, as it's only used during error messages, and in most instances cases is actually set to "command line". Rename it to "config_source" to make it clear that it's not used to actually open any files. No functional changes. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_cfg.c --- a/tools/libxl/libxlu_cfg.c Tue May 15 15:30:32 2012 +0100 +++ b/tools/libxl/libxlu_cfg.c Tue May 15 15:45:55 2012 +0100 @@ -25,15 +25,15 @@ #include "libxlu_cfg_l.h" #include "libxlu_cfg_i.h" -XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename) { +XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) { XLU_Config *cfg; cfg= malloc(sizeof(*cfg)); if (!cfg) return 0; cfg->report= report; - cfg->filename= strdup(report_filename); - if (!cfg->filename) { free(cfg); return 0; } + cfg->config_source= strdup(report_source); + if (!cfg->config_source) { free(cfg); return 0; } cfg->settings= 0; return cfg; @@ -51,7 +51,7 @@ static int ctx_prep(CfgParseContext *ctx e= xlu__cfg_yylex_init_extra(ctx, &ctx->scanner); if (e) { fprintf(cfg->report,"%s: unable to create scanner: %s\n", - cfg->filename, strerror(e)); + cfg->config_source, strerror(e)); return e; } return 0; @@ -117,7 +117,7 @@ int xlu_cfg_readdata(XLU_Config *cfg, co buf = xlu__cfg_yy_scan_bytes(data, length, ctx.scanner); if (!buf) { fprintf(cfg->report,"%s: unable to allocate scanner buffer\n", - cfg->filename); + cfg->config_source); ctx.err= ENOMEM; goto xe; } @@ -151,7 +151,7 @@ void xlu_cfg_destroy(XLU_Config *cfg) { set_next= set->next; xlu__cfg_set_free(set); } - free(cfg->filename); + free(cfg->config_source); free(cfg); } @@ -178,7 +178,7 @@ static int find_atom(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is" " a list but should be a single value\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *set_r= set; @@ -223,14 +223,14 @@ int xlu_cfg_get_long(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' could not be parsed" " as a number: %s\n", - cfg->filename, set->lineno, n, strerror(e)); + cfg->config_source, set->lineno, n, strerror(e)); return e; } if (*ep || ep==set->values[0]) { if (!dont_warn) fprintf(cfg->report, "%s:%d: warning: parameter `%s' is not a valid number\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); return EINVAL; } *value_r= l; @@ -258,7 +258,7 @@ int xlu_cfg_get_list(const XLU_Config *c fprintf(cfg->report, "%s:%d: warning: parameter `%s' is a single value" " but should be a list\n", - cfg->filename, set->lineno, n); + cfg->config_source, set->lineno, n); } return EINVAL; } @@ -467,7 +467,7 @@ void xlu__cfg_yyerror(YYLTYPE *loc, CfgP fprintf(ctx->cfg->report, "%s:%d: config parsing error near %s%.*s%s%s: %s\n", - ctx->cfg->filename, lineno, + ctx->cfg->config_source, lineno, len?"`":"", len, text, len?"'":"", newline, msg); if (!ctx->err) ctx->err= EINVAL; diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_disk.c --- a/tools/libxl/libxlu_disk.c Tue May 15 15:30:32 2012 +0100 +++ b/tools/libxl/libxlu_disk.c Tue May 15 15:45:55 2012 +0100 @@ -10,7 +10,7 @@ void xlu__disk_err(DiskParseContext *dpc "%s: config parsing error in disk specification: %s" "%s%s%s" " in `%s'\n", - dpc->cfg->filename, message, + dpc->cfg->config_source, message, erroneous?": near `":"", erroneous?erroneous:"", erroneous?"'":"", dpc->spec); if (!dpc->err) dpc->err= EINVAL; diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_internal.h --- a/tools/libxl/libxlu_internal.h Tue May 15 15:30:32 2012 +0100 +++ b/tools/libxl/libxlu_internal.h Tue May 15 15:45:55 2012 +0100 @@ -38,7 +38,7 @@ struct XLU_ConfigSetting { /* transparen struct XLU_Config { XLU_ConfigSetting *settings; FILE *report; - char *filename; + char *config_source; }; typedef struct { diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_vif.c --- a/tools/libxl/libxlu_vif.c Tue May 15 15:30:32 2012 +0100 +++ b/tools/libxl/libxlu_vif.c Tue May 15 15:45:55 2012 +0100 @@ -7,7 +7,7 @@ static const char *vif_internal_usec_re static void xlu__vif_err(XLU_Config *cfg, const char *msg, const char *rate) { fprintf(cfg->report, "%s: config parsing error in vif: %s in `%s'\n", - cfg->filename, msg, rate); + cfg->config_source, msg, rate); } static int vif_parse_rate_bytes_per_sec(XLU_Config *cfg, const char *bytes, ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-15 14:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <patchbomb.1333477720@kodo2>
2012-04-03 18:28 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap
2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
2012-04-03 18:52 ` George Dunlap
2012-04-04 15:18 ` Ian Jackson
[not found] <patchbomb.1335971421@kodo2>
2012-05-02 15:10 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap
2012-05-15 14:51 [PATCH 0 of 2] libxl cleanup: distinguish filenames from sources George Dunlap
2012-05-15 14:51 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).