Commit 89c50775 authored by Daniel Salzman's avatar Daniel Salzman

libknot/dname: code cleanup

parent 31e3c38b
......@@ -100,11 +100,7 @@ static int get_zone(ctl_args_t *args, zone_t **zone)
if (dname == NULL) {
return KNOT_EINVAL;
}
int ret = knot_dname_to_lower(dname);
if (ret != KNOT_EOK) {
return ret;
}
knot_dname_to_lower(dname);
*zone = knot_zonedb_find(args->server->zone_db, dname);
if (*zone == NULL) {
......@@ -592,11 +588,7 @@ static int get_owner(uint8_t *out, size_t out_len, knot_dname_t *origin,
if (dname == NULL) {
return KNOT_EINVAL;
}
int ret = knot_dname_to_lower(dname);
if (ret != KNOT_EOK) {
return ret;
}
knot_dname_to_lower(dname);
prefix_len = knot_dname_size(out);
if (prefix_len == 0) {
......
......@@ -67,7 +67,7 @@ static int deserialize_rrset(wire_ctx_t *wire, knot_rrset_t *rrset, long *phase)
if (*phase == SERIALIZE_RRSET_INIT && wire_ctx_available(wire) > 0) {
// Read owner, rtype, rclass and RR count.
size_t size = knot_dname_size(wire->position);
knot_dname_t *owner = knot_dname_copy_part(wire->position, size, NULL);
knot_dname_t *owner = knot_dname_copy(wire->position, NULL);
if (owner == NULL || wire_ctx_available(wire) < size + 3 * sizeof(uint16_t)) {
return KNOT_EMALF;
}
......
......@@ -74,7 +74,7 @@ static int dname_cname_synth(const knot_rrset_t *dname_rr,
const knot_dname_t *dname_wire = dname_rr->owner;
const knot_dname_t *dname_tgt = knot_dname_target(&dname_rr->rrs);
size_t labels = knot_dname_labels(dname_wire, NULL);
knot_dname_t *cname = knot_dname_replace_suffix(qname, labels, dname_tgt);
knot_dname_t *cname = knot_dname_replace_suffix(qname, labels, dname_tgt, mm);
if (cname == NULL) {
knot_dname_free(&owner_copy, mm);
return KNOT_ENOMEM;
......@@ -84,7 +84,7 @@ static int dname_cname_synth(const knot_rrset_t *dname_rr,
size_t cname_size = knot_dname_size(cname);
uint8_t cname_rdata[cname_size];
memcpy(cname_rdata, cname, cname_size);
knot_dname_free(&cname, NULL);
knot_dname_free(&cname, mm);
int ret = knot_rrset_add_rdata(cname_rrset, cname_rdata, cname_size, mm);
if (ret != KNOT_EOK) {
......
/* Copyright (C) 2017 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
/* Copyright (C) 2018 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -320,7 +320,7 @@ static int answer_edns_put(knot_pkt_t *resp, knotd_qdata_t *qdata)
}
/*! \brief Initialize response, sizes and find zone from which we're going to answer. */
static int prepare_answer(const knot_pkt_t *query, knot_pkt_t *resp, knot_layer_t *ctx)
static int prepare_answer(knot_pkt_t *query, knot_pkt_t *resp, knot_layer_t *ctx)
{
knotd_qdata_t *qdata = QUERY_DATA(ctx);
server_t *server = qdata->params->server;
......@@ -343,10 +343,8 @@ static int prepare_answer(const knot_pkt_t *query, knot_pkt_t *resp, knot_layer_
* Already checked for absence of compression and length.
*/
memcpy(qdata->extra->orig_qname, qname, query->qname_size);
ret = process_query_qname_case_lower((knot_pkt_t *)query);
if (ret != KNOT_EOK) {
return ret;
}
process_query_qname_case_lower(query);
/* Find zone for QNAME. */
qdata->extra->zone = answer_zone_find(query, server->zone_db);
......@@ -735,22 +733,6 @@ fail:
return ret;
}
void process_query_qname_case_restore(knot_pkt_t *pkt, knotd_qdata_t *qdata)
{
/* If original QNAME is empty, Query is either unparsed or for root domain.
* Either way, letter case doesn't matter. */
if (qdata->extra->orig_qname[0] != '\0') {
memcpy(pkt->wire + KNOT_WIRE_HEADER_SIZE,
qdata->extra->orig_qname, qdata->query->qname_size);
}
}
int process_query_qname_case_lower(knot_pkt_t *pkt)
{
knot_dname_t *qname = (knot_dname_t *)knot_pkt_qname(pkt);
return knot_dname_to_lower(qname);
}
/*! \brief Synthesize RRSIG for given parameters, store in 'qdata' for later use */
static int put_rrsig(const knot_dname_t *sig_owner, uint16_t type,
const knot_rrset_t *rrsigs, knot_rrinfo_t *rrinfo,
......
/* Copyright (C) 2017 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
/* Copyright (C) 2018 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -97,14 +97,24 @@ int process_query_sign_response(knot_pkt_t *pkt, knotd_qdata_t *qdata);
* \param pkt Incoming message.
* \param qdata Query data.
*/
void process_query_qname_case_restore(knot_pkt_t *pkt, knotd_qdata_t *qdata);
static inline void process_query_qname_case_restore(knot_pkt_t *pkt, knotd_qdata_t *qdata)
{
// If original QNAME is empty, query is either unparsed or for root domain.
if (qdata->extra->orig_qname[0] != '\0') {
memcpy(pkt->wire + KNOT_WIRE_HEADER_SIZE,
qdata->extra->orig_qname, qdata->query->qname_size);
}
}
/*!
* \brief Convert QNAME to lowercase format for processing.
*
* \param pkt Incoming message.
*/
int process_query_qname_case_lower(knot_pkt_t *pkt);
static inline void process_query_qname_case_lower(knot_pkt_t *pkt)
{
knot_dname_to_lower(knot_pkt_qname(pkt));
}
/*!
* \brief Puts RRSet to packet, will store its RRSIG for later use.
......
......@@ -33,32 +33,59 @@ static int label_is_equal(const uint8_t *lb1, const uint8_t *lb2)
return (*lb1 == *lb2) && memcmp(lb1 + 1, lb2 + 1, *lb1) == 0;
}
/*!
* \brief Align name end-to-end and return number of common suffix labels.
*
* \param d1 Domain name1.
* \param d1_labels Number of labels in d1.
* \param d2 Domain name2.
* \param d2_labels Number of labels in d2.
*/
static int dname_align(const uint8_t **d1, uint8_t d1_labels,
const uint8_t **d2, uint8_t d2_labels)
{
assert(d1 && d2);
for (unsigned j = d1_labels; j < d2_labels; ++j) {
*d2 = knot_wire_next_label(*d2, NULL);
}
for (unsigned j = d2_labels; j < d1_labels; ++j) {
*d1 = knot_wire_next_label(*d1, NULL);
}
return (d1_labels < d2_labels) ? d1_labels : d2_labels;
}
_public_
int knot_dname_wire_check(const uint8_t *name, const uint8_t *endp,
const uint8_t *pkt)
{
if (name == NULL || name == endp)
return KNOT_EMALF;
if (name == NULL || name == endp) {
return KNOT_EINVAL;
}
int wire_len = 0;
int name_len = 1; /* Keep \x00 terminal label in advance. */
bool is_compressed = false;
while (*name != '\0') {
/* Check bounds (must have at least 2 octets remaining). */
if (name + 2 > endp)
return KNOT_ESPACE;
if (name + 2 > endp) {
return KNOT_EMALF;
}
if (knot_wire_is_pointer(name)) {
/* Check that the pointer points backwards
* otherwise it could result in infinite loop
*/
if (pkt == NULL)
if (pkt == NULL) {
return KNOT_EINVAL;
}
uint16_t ptr = knot_wire_get_pointer(name);
if (ptr >= (name - pkt))
if (ptr >= (name - pkt)) {
return KNOT_EMALF;
}
name = pkt + ptr; /* Hop to compressed label */
if (!is_compressed) { /* Measure compressed size only */
......@@ -67,27 +94,32 @@ int knot_dname_wire_check(const uint8_t *name, const uint8_t *endp,
}
} else {
/* Check label length. */
if (*name > KNOT_DNAME_MAXLABELLEN)
if (*name > KNOT_DNAME_MAXLABELLEN) {
return KNOT_EMALF;
}
/* Check if there's enough space. */
int lblen = *name + 1;
if (name_len + lblen > KNOT_DNAME_MAXLEN)
if (name_len + lblen > KNOT_DNAME_MAXLEN) {
return KNOT_EMALF;
}
/* Update wire size only for noncompressed part. */
name_len += lblen;
if (!is_compressed)
if (!is_compressed) {
wire_len += lblen;
}
/* Hop to next label. */
name += lblen;
}
/* Check bounds (must have at least 1 octet). */
if (name + 1 > endp)
return KNOT_ESPACE;
if (name + 1 > endp) {
return KNOT_EMALF;
}
}
if (!is_compressed) /* Terminal label. */
if (!is_compressed) { /* Terminal label. */
wire_len += 1;
}
return wire_len;
}
......@@ -96,8 +128,9 @@ _public_
knot_dname_t *knot_dname_parse(const uint8_t *pkt, size_t *pos, size_t maxpos,
knot_mm_t *mm)
{
if (pkt == NULL || pos == NULL)
if (pkt == NULL || pos == NULL) {
return NULL;
}
const uint8_t *name = pkt + *pos;
const uint8_t *endp = pkt + maxpos;
......@@ -114,7 +147,7 @@ knot_dname_t *knot_dname_parse(const uint8_t *pkt, size_t *pos, size_t maxpos,
/* Allocate space for the name. */
knot_dname_t *res = mm_alloc(mm, decompressed_len);
if (res) {
if (res != NULL) {
/* Unpack name (expand compression pointers). */
if (knot_dname_unpack(res, name, decompressed_len, pkt) > 0) {
*pos += parsed;
......@@ -128,26 +161,32 @@ knot_dname_t *knot_dname_parse(const uint8_t *pkt, size_t *pos, size_t maxpos,
}
_public_
knot_dname_t *knot_dname_copy(const knot_dname_t *name, knot_mm_t *mm)
size_t knot_dname_store(knot_dname_storage_t *dst, const knot_dname_t *name)
{
if (name == NULL)
return NULL;
if (dst == NULL || name == NULL) {
return 0;
}
return knot_dname_copy_part(name, knot_dname_size(name), mm);
size_t len = knot_dname_size(name);
assert(len <= KNOT_DNAME_MAXLEN);
memcpy(dst->data, name, len);
return len;
}
_public_
knot_dname_t *knot_dname_copy_part(const knot_dname_t *name, unsigned len,
knot_mm_t *mm)
knot_dname_t *knot_dname_copy(const knot_dname_t *name, knot_mm_t *mm)
{
if (name == NULL || len == 0)
if (name == NULL) {
return NULL;
}
size_t len = knot_dname_size(name);
knot_dname_t *dst = mm_alloc(mm, len);
if (knot_dname_to_wire(dst, name, len) < 1) {
mm_free(mm, dst);
if (dst == NULL) {
return NULL;
}
memcpy(dst, name, len);
return dst;
}
......@@ -163,8 +202,8 @@ int knot_dname_to_wire(uint8_t *dst, const knot_dname_t *src, size_t maxlen)
if (len > maxlen) {
return KNOT_ESPACE;
}
memcpy(dst, src, len);
return len;
}
......@@ -172,8 +211,9 @@ _public_
int knot_dname_unpack(uint8_t *dst, const knot_dname_t *src,
size_t maxlen, const uint8_t *pkt)
{
if (dst == NULL || src == NULL)
if (dst == NULL || src == NULL) {
return KNOT_EINVAL;
}
/* Seek first real label occurrence. */
src = knot_wire_seek_label(src, pkt);
......@@ -182,16 +222,18 @@ int knot_dname_unpack(uint8_t *dst, const knot_dname_t *src,
int len = 0;
while (*src != '\0') {
uint8_t lblen = *src + 1;
if (len + lblen > maxlen)
if (len + lblen > maxlen) {
return KNOT_ESPACE;
}
memcpy(dst + len, src, lblen);
len += lblen;
src = knot_wire_next_label(src, pkt);
}
/* Terminal label */
if (len + 1 > maxlen)
if (len + 1 > maxlen) {
return KNOT_EINVAL;
}
*(dst + len) = '\0';
return len + 1;
......@@ -441,23 +483,19 @@ dname_from_str_failed:
}
_public_
int knot_dname_to_lower(knot_dname_t *name)
void knot_dname_to_lower(knot_dname_t *name)
{
if (name == NULL)
return KNOT_EINVAL;
if (name == NULL) {
return;
}
while (*name != '\0') {
uint8_t len = *name;
for (uint8_t i = 1; i <= len; ++i) {
name[i] = knot_tolower(name[i]);
}
name = (uint8_t *)knot_wire_next_label(name, NULL);
if (name == NULL) { /* Must not be used on compressed names. */
return KNOT_EMALF;
}
name += 1 + len;
}
return KNOT_EOK;
}
_public_
......@@ -519,7 +557,7 @@ bool knot_dname_is_sub(const knot_dname_t *sub, const knot_dname_t *domain)
}
/* Align end-to-end to common suffix. */
int common = knot_dname_align(&sub, sub_l, &domain, domain_l, NULL);
int common = dname_align(&sub, sub_l, &domain, domain_l);
/* Compare common suffix. */
while (common > 0) {
......@@ -532,6 +570,7 @@ bool knot_dname_is_sub(const knot_dname_t *sub, const knot_dname_t *domain)
domain = knot_wire_next_label(domain, NULL);
--common;
}
return true;
}
......@@ -541,16 +580,6 @@ bool knot_dname_in(const knot_dname_t *domain, const knot_dname_t *sub)
return knot_dname_is_equal(domain, sub) || knot_dname_is_sub(sub, domain);
}
_public_
bool knot_dname_is_wildcard(const knot_dname_t *name)
{
if (name == NULL) {
return false;
}
return name[0] == 1 && name[1] == '*';
}
_public_
size_t knot_dname_matched_labels(const knot_dname_t *d1, const knot_dname_t *d2)
{
......@@ -562,7 +591,7 @@ size_t knot_dname_matched_labels(const knot_dname_t *d1, const knot_dname_t *d2)
}
/* Align end-to-end to common suffix. */
int common = knot_dname_align(&d1, l1, &d2, l2, NULL);
int common = dname_align(&d1, l1, &d2, l2);
/* Count longest chain leading to root label. */
size_t matched = 0;
......@@ -584,7 +613,7 @@ size_t knot_dname_matched_labels(const knot_dname_t *d1, const knot_dname_t *d2)
_public_
knot_dname_t *knot_dname_replace_suffix(const knot_dname_t *name, unsigned labels,
const knot_dname_t *suffix)
const knot_dname_t *suffix, knot_mm_t *mm)
{
if (name == NULL) {
return NULL;
......@@ -605,7 +634,7 @@ knot_dname_t *knot_dname_replace_suffix(const knot_dname_t *name, unsigned label
/* Create target name. */
size_t new_len = prefix_len + suffix_len;
knot_dname_t *out = malloc(new_len);
knot_dname_t *out = mm_alloc(mm, new_len);
if (out == NULL) {
return NULL;
}
......@@ -625,6 +654,7 @@ knot_dname_t *knot_dname_replace_suffix(const knot_dname_t *name, unsigned label
dst += *suffix + 1;
suffix = knot_wire_next_label(suffix, NULL);
}
*dst = '\0';
return out;
}
......@@ -632,8 +662,9 @@ knot_dname_t *knot_dname_replace_suffix(const knot_dname_t *name, unsigned label
_public_
void knot_dname_free(knot_dname_t **name, knot_mm_t *mm)
{
if (name == NULL || *name == NULL)
if (name == NULL || *name == NULL) {
return;
}
mm_free(mm, *name);
*name = NULL;
......@@ -717,22 +748,6 @@ bool knot_dname_label_is_equal(const uint8_t *label1, const uint8_t *label2)
return true;
}
_public_
knot_dname_t *knot_dname_cat(knot_dname_t *d1, const knot_dname_t *d2)
{
if (d1 == NULL || d2 == NULL)
return NULL;
/* This is problem equal to replacing last \x00 from d1 with d2. */
knot_dname_t *ret = knot_dname_replace_suffix(d1, 0, d2);
/* Like if we are reallocating d1. */
if (ret != NULL)
knot_dname_free(&d1, NULL);
return ret;
}
_public_
size_t knot_dname_prefixlen(const uint8_t *name, unsigned nlabels, const uint8_t *pkt)
{
......@@ -775,24 +790,8 @@ size_t knot_dname_labels(const uint8_t *name, const uint8_t *pkt)
return 0;
}
}
return count;
}
_public_
int knot_dname_align(const uint8_t **d1, uint8_t d1_labels,
const uint8_t **d2, uint8_t d2_labels,
uint8_t *wire)
{
if (d1 == NULL || d2 == NULL)
return KNOT_EINVAL;
for (unsigned j = d1_labels; j < d2_labels; ++j)
*d2 = knot_wire_next_label(*d2, wire);
for (unsigned j = d2_labels; j < d1_labels; ++j)
*d1 = knot_wire_next_label(*d1, wire);
return (d1_labels < d2_labels) ? d1_labels : d2_labels;
return count;
}
_public_
......
This diff is collapsed.
......@@ -626,8 +626,6 @@ int knot_pkt_parse_question(knot_pkt_t *pkt)
return KNOT_EMALF; \
} else if (!check_func(rr)) { \
return KNOT_EMALF; \
} else { \
(pkt)->var = rr; \
}
/*! \brief Check constraints (position, uniqueness, validity) for special types
......@@ -640,6 +638,7 @@ static int check_rr_constraints(knot_pkt_t *pkt, knot_rrset_t *rr, size_t rr_siz
switch (rr->type) {
case KNOT_RRTYPE_TSIG:
CHECK_AR_CONSTRAINTS(pkt, rr, tsig_rr, knot_tsig_rdata_is_ok);
pkt->tsig_rr = rr;
/* Strip TSIG RR from wireformat and decrease ARCOUNT. */
if (!(flags & KNOT_PF_KEEPWIRE)) {
......@@ -652,6 +651,7 @@ static int check_rr_constraints(knot_pkt_t *pkt, knot_rrset_t *rr, size_t rr_siz
break;
case KNOT_RRTYPE_OPT:
CHECK_AR_CONSTRAINTS(pkt, rr, opt_rr, knot_edns_check_record);
pkt->opt_rr = rr;
break;
default:
break;
......
......@@ -169,7 +169,7 @@ static inline uint16_t knot_pkt_question_size(const knot_pkt_t *pkt)
return pkt->qname_size + 2 * sizeof(uint16_t);
}
static inline const knot_dname_t *knot_pkt_qname(const knot_pkt_t *pkt)
static inline knot_dname_t *knot_pkt_qname(const knot_pkt_t *pkt)
{
if (pkt == NULL || pkt->qname_size == 0) {
return NULL;
......
......@@ -185,10 +185,7 @@ int knot_rrset_rr_to_canonical(knot_rrset_t *rrset)
}
/* Convert owner for all RRSets. */
int ret = knot_dname_to_lower(rrset->owner);
if (ret != KNOT_EOK) {
return ret;
}
knot_dname_to_lower(rrset->owner);
/* Convert DNAMEs in RDATA only for RFC4034 types. */
if (!knot_rrtype_should_be_lowercased(rrset->type)) {
......@@ -218,16 +215,11 @@ int knot_rrset_rr_to_canonical(knot_rrset_t *rrset)
case KNOT_RDATA_WF_COMPRESSIBLE_DNAME:
case KNOT_RDATA_WF_DECOMPRESSIBLE_DNAME:
case KNOT_RDATA_WF_FIXED_DNAME:
ret = knot_dname_to_lower(pos);
if (ret != KNOT_EOK) {
return ret;
}
ret = knot_dname_size(pos);
pos += ret;
knot_dname_to_lower(pos);
pos += knot_dname_size(pos);
break;
case KNOT_RDATA_WF_NAPTR_HEADER:
ret = knot_naptr_header_size(pos, endpos);
; int ret = knot_naptr_header_size(pos, endpos);
if (ret < 0) {
return ret;
}
......
......@@ -170,9 +170,7 @@ static int write_tsig_variables(uint8_t *wire, const knot_rrset_t *tsig_rr)
/* Te algorithm name must be in canonical form, i.e. in lowercase. */
uint8_t *alg_name_wire = wire + offset;
offset += knot_dname_to_wire(alg_name_wire, alg_name, KNOT_DNAME_MAXLEN);
if (knot_dname_to_lower(alg_name_wire) != KNOT_EOK) {
return KNOT_EINVAL;
}
knot_dname_to_lower(alg_name_wire);
/* Following data are written in network order. */
/* Time signed. */
......
......@@ -704,9 +704,7 @@ int yp_dname_to_bin(
}
// Convert the result to lower case.
if (knot_dname_to_lower(out->position) != KNOT_EOK) {
return KNOT_EINVAL;
}
knot_dname_to_lower(out->position);
wire_ctx_skip(out, ret);
......
/* Copyright (C) 2017 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
/* Copyright (C) 2018 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -601,7 +601,7 @@ int keymgr_foreign_key_id(char *argv[], knot_dname_t **key_zone, char **key_id)
if (*key_zone == NULL) {
return KNOT_ENOMEM;
}
(void)knot_dname_to_lower(*key_zone);
knot_dname_to_lower(*key_zone);
kdnssec_ctx_t kctx = { 0 };
int ret = kdnssec_ctx_init(conf(), &kctx, *key_zone, NULL);
......
/* Copyright (C) 2017 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
/* Copyright (C) 2018 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -95,7 +95,7 @@ static int key_command(int argc, char *argv[], int optind)
if (zone_name == NULL) {
return KNOT_ENOMEM;
}
(void)knot_dname_to_lower(zone_name);
knot_dname_to_lower(zone_name);
kdnssec_ctx_t kctx = { 0 };
......
......@@ -515,12 +515,12 @@ static int zone_exec(cmd_args_t *args, int (*fcn)(const knot_dname_t *, void *),
uint8_t id[KNOT_DNAME_MAXLEN];
for (int i = 0; i < args->argc; i++) {
if (knot_dname_from_str(id, args->argv[i], sizeof(id)) == NULL ||
knot_dname_to_lower(id) != KNOT_EOK) {
if (knot_dname_from_str(id, args->argv[i], sizeof(id)) == NULL) {
log_zone_str_error(args->argv[i], "invalid name");
failed = true;
continue;
}
knot_dname_to_lower(id);
if (!conf_rawid_exists(conf(), C_ZONE, id, knot_dname_size(id))) {
log_zone_error(id, "%s", knot_strerror(KNOT_ENOZONE));
......
......@@ -18,7 +18,6 @@
#include <string.h>
#include <tap/basic.h>
#include "libknot/consts.h"
#include "libknot/dname.h"
/* Test dname_parse_from_wire */
......@@ -513,21 +512,6 @@ int main(int argc, char *argv[])
knot_dname_free(&d, NULL);
knot_dname_free(&d2, NULL);
/* DNAME CAT CHECKS */
/* dname cat (valid) */
w = "\x03""cat";
d = knot_dname_copy((const uint8_t *)w, NULL);
t = "*";
d2 = knot_dname_from_str_alloc(t);
d2 = knot_dname_cat(d2, d);
t = "\x01""*""\x03""cat";
len = 2 + 4 + 1;
ok(d2 && len == knot_dname_size(d2), "dname_cat: valid concatenation size");
ok(d2 && memcmp(d2, t, len) == 0, "dname_cat: valid concatenation");
knot_dname_free(&d, NULL);
knot_dname_free(&d2, NULL);
/* DNAME PARSE CHECKS */
/* parse from wire (valid) */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment