Attention is currently required from: Nico Huber, Daniel Campello, Angel Pons.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52362 )
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
Patch Set 18: -Code-Review
(1 comment)
Patchset:
PS18:
Daniel, can we go the way of Nico's idea? Like I mention out of band a week or so ago I was never a huge fan of the i/o op argument being optional. I think Nico's mode concept is much more robust, we would need to fix the call sites in our tree but I think its worth it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 08 May 2021 00:31:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52794 )
Change subject: tests: Start port-i/o mocking framework
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> Just wanted to say, it would be great to merge this if the Owner agrees (and if everyone agrees) - t […]
You can also rebase on unmerged patches. In this case, it would have
made the dependencies visible. IOW, it's harder to miss a change that's
pending merge when it's part of a relation chain. ;)
--
To view, visit https://review.coreboot.org/c/flashrom/+/52794
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8df02832deba80761b57435244a29d0d9b4e2649
Gerrit-Change-Number: 52794
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 07 May 2021 11:36:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52794 )
Change subject: tests: Start port-i/o mocking framework
......................................................................
tests: Start port-i/o mocking framework
This will be used to mock the i/o needs of indvidiual programmer
drivers. For each programmer driver, a `struct io_mock` can be
registered to dispatch inb()/outb() and friends.
Change-Id: I8df02832deba80761b57435244a29d0d9b4e2649
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52794
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
A tests/io_mock.h
1 file changed, 49 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, but someone else must approve
Edward O'Callaghan: Looks good to me, approved
Anastasia Klimchuk: Looks good to me, but someone else must approve
diff --git a/tests/io_mock.h b/tests/io_mock.h
new file mode 100644
index 0000000..056e6bf
--- /dev/null
+++ b/tests/io_mock.h
@@ -0,0 +1,49 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright (c) 2021 Nico Huber <nico.h(a)gmx.de>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the author nor the names of its contributors
+ * may be used to endorse or promote products derived from this
+ * software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef _IO_MOCK_H_
+#define _IO_MOCK_H_
+
+struct io_mock {
+ void *priv;
+
+ void (*outb)(void *priv, unsigned char value, unsigned short port);
+ unsigned char (*inb)(void *priv, unsigned short port);
+
+ void (*outw)(void *priv, unsigned short value, unsigned short port);
+ unsigned short (*inw)(void *priv, unsigned short port);
+
+ void (*outl)(void *priv, unsigned int value, unsigned short port);
+ unsigned int (*inl)(void *priv, unsigned short port);
+};
+
+void io_mock_register(const struct io_mock *io);
+
+#endif
--
To view, visit https://review.coreboot.org/c/flashrom/+/52794
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8df02832deba80761b57435244a29d0d9b4e2649
Gerrit-Change-Number: 52794
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52794 )
Change subject: tests: Start port-i/o mocking framework
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Just wanted to say, it would be great to merge this if the Owner agrees (and if everyone agrees) - then I can rebase CB:51487 on the top and make it ready for further review. Thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/52794
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8df02832deba80761b57435244a29d0d9b4e2649
Gerrit-Change-Number: 52794
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Fri, 07 May 2021 05:44:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52362 )
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
Patch Set 18:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/52362/comment/06e45579_66cdab7e
PS17, Line 496: (read_it | write_it | verify_it) &&
> Copying from CB:30979 I guess this is not needed since that would only be true if filename
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 07 May 2021 02:53:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52362
to look at the new patch set (#18).
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
cli_classic.c: Make -r/-w/-v argument optional
Makes filename optional from -r/-w/-v since the -i parameter allows
building the image to be written from multiple files, and regions to be
read from flash and written to separate image files,
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
---
M cli_classic.c
M flashrom.c
2 files changed, 86 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/52362/18
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52892 )
Change subject: flashrom.8.tmpl: Add man entry for --extract
......................................................................
flashrom.8.tmpl: Add man entry for --extract
This is a follow up change of CB:52450
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: Icc068f5545b6f30ac390b7b815a31e2d61bf4789
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52892
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M flashrom.8.tmpl
1 file changed, 5 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 4634539..4ae7e71 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -47,7 +47,7 @@
.B flashrom \fR[\fB\-h\fR|\fB\-R\fR|\fB\-L\fR|\fB\-z\fR|
\fB\-p\fR <programmername>[:<parameters>] [\fB\-c\fR <chipname>]
(\fB\-\-flash\-name\fR|\fB\-\-flash\-size\fR|
- [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>]
+ [\fB\-E\fR|\fB\-x\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>]
[(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>)
[\fB\-i\fR <image>[:<file>]]]
[\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])]
@@ -136,6 +136,10 @@
.B "\-E, \-\-erase"
Erase the flash ROM chip.
.TP
+.B "\-x, \-\-extract"
+Extract every region defined on the layout from flash ROM chip to a
+file with the same name as the extracted region.
+.TP
.B "\-V, \-\-verbose"
More verbose output. This option can be supplied multiple times
(max. 3 times, i.e.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52892
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icc068f5545b6f30ac390b7b815a31e2d61bf4789
Gerrit-Change-Number: 52892
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/50246 )
Change subject: programmer.h: Convert anon union to anon struct
......................................................................
programmer.h: Convert anon union to anon struct
Convert the anon union of registered masters in the mst
field of the flashctx to a anon struct. If we are going
to dereference a pointer there in an undefined way we
should crash and not plow ahead with invalid memory.
The user of the registered_masters type is therefore
responsible for querying the buses_supported field before
attempting to dereference a ptr field in the anon struct.
BUG=b:175849641
TEST=`flashrom -p internal --flash-name`
Change-Id: I576967a8599b923c902e39f177f39146291cc242
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/50246
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-by: Daniel Campello <campello(a)chromium.org>
Reviewed-by: Sam McNally <sammc(a)google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M opaque.c
M programmer.c
M programmer.h
M spi.c
4 files changed, 5 insertions(+), 4 deletions(-)
Approvals:
build bot (Jenkins): Verified
Sam McNally: Looks good to me, approved
Daniel Campello: Looks good to me, but someone else must approve
Peter Marheine: Looks good to me, but someone else must approve
Anastasia Klimchuk: Looks good to me, but someone else must approve
diff --git a/opaque.c b/opaque.c
index 276934f..e3103c8 100644
--- a/opaque.c
+++ b/opaque.c
@@ -48,7 +48,7 @@
int register_opaque_master(const struct opaque_master *mst)
{
- struct registered_master rmst;
+ struct registered_master rmst = {0};
if (!mst->probe || !mst->read || !mst->write || !mst->erase) {
msg_perr("%s called with incomplete master definition. "
diff --git a/programmer.c b/programmer.c
index bee60e3..42ea2e3 100644
--- a/programmer.c
+++ b/programmer.c
@@ -83,7 +83,8 @@
int register_par_master(const struct par_master *mst,
const enum chipbustype buses)
{
- struct registered_master rmst;
+ struct registered_master rmst = {0};
+
if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel ||
!mst->chip_writen || !mst->chip_readb || !mst->chip_readw ||
!mst->chip_readl || !mst->chip_readn) {
diff --git a/programmer.h b/programmer.h
index 29a100b..675a259 100644
--- a/programmer.h
+++ b/programmer.h
@@ -749,7 +749,7 @@
int register_par_master(const struct par_master *mst, const enum chipbustype buses);
struct registered_master {
enum chipbustype buses_supported;
- union {
+ struct {
struct par_master par;
struct spi_master spi;
struct opaque_master opaque;
diff --git a/spi.c b/spi.c
index aed2a92..aa245d7 100644
--- a/spi.c
+++ b/spi.c
@@ -133,7 +133,7 @@
int register_spi_master(const struct spi_master *mst)
{
- struct registered_master rmst;
+ struct registered_master rmst = {0};
if (!mst->write_aai || !mst->write_256 || !mst->read || !mst->command ||
!mst->multicommand ||
--
To view, visit https://review.coreboot.org/c/flashrom/+/50246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242
Gerrit-Change-Number: 50246
Gerrit-PatchSet: 5
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52362 )
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
Patch Set 17:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/52362/comment/dcc4e3ef_f9b872f7
PS17, Line 496: (read_it | write_it | verify_it) &&
Copying from CB:30979 I guess this is not needed since that would only be true if filename
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 17
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 07 May 2021 01:20:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment