diff mbox series

[2/4] nbd/server: add nbd_meta_single_query helper

Message ID 20180321121940.39426-3-vsementsov@virtuozzo.com
State New
Headers show
Series NBD export bitmaps | expand

Commit Message

Vladimir Sementsov-Ogievskiy March 21, 2018, 12:19 p.m. UTC
The helper will be reused for bitmaps namespace.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

Comments

Eric Blake March 21, 2018, 3:05 p.m. UTC | #1
On 03/21/2018 07:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> The helper will be reused for bitmaps namespace.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   nbd/server.c | 41 ++++++++++++++++++++++++++++-------------
>   1 file changed, 28 insertions(+), 13 deletions(-)
> 

> +/* Read len bytes and check matching to the pattern.
> + * @match is set to true on empty query for _LIST_ and for query matching the
> + * @pattern. @match is never set to false.

How about:

Read @len bytes, and set @match to true if they match @pattern, or if 
@len is 0 and the client is performing _LIST_.  @match is never set to 
false.

At any rate, the refactoring is sane; and comment touchups are trivial, so

Reviewed-by: Eric Blake <eblake@redhat.com>
Vladimir Sementsov-Ogievskiy April 13, 2018, 5:44 p.m. UTC | #2
21.03.2018 18:05, Eric Blake wrote:
> On 03/21/2018 07:19 AM, Vladimir Sementsov-Ogievskiy wrote:
>> The helper will be reused for bitmaps namespace.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   nbd/server.c | 41 ++++++++++++++++++++++++++++-------------
>>   1 file changed, 28 insertions(+), 13 deletions(-)
>>
>
>> +/* Read len bytes and check matching to the pattern.
>> + * @match is set to true on empty query for _LIST_ and for query 
>> matching the
>> + * @pattern. @match is never set to false.
>
> How about:
>
> Read @len bytes, and set @match to true if they match @pattern, or if 
> @len is 0 and the client is performing _LIST_.  @match is never set to 
> false.

will add, as always, thank you for natural rewording) Hm, I have a 
question: why do you often use double white-space "  " between 
sentences? Is it something meaningful?

>
> At any rate, the refactoring is sane; and comment touchups are 
> trivial, so
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
John Snow April 13, 2018, 9:06 p.m. UTC | #3
On 04/13/2018 01:44 PM, Vladimir Sementsov-Ogievskiy wrote:
> 
> will add, as always, thank you for natural rewording) Hm, I have a
> question: why do you often use double white-space "  " between
> sentences? Is it something meaningful?

There is some GREAT DEBATE in the English-speaking world over whether or
not this is correct typography. At one point, it was thought that there
should be two spaces after every full stop (".") to improve readability.
Allegedly, this was most important for physical typesetting on typewriters.

Since digital typography has taken off, some people argue that this is a
relic and that semantically we ought to be using only one literal space
after the full stop, and using various kerning and display parameters to
extend the physical buffer between two sentences if desired.

Famously in my mind, PEP8 mandates the two spaces after a period style.

MLA says "One, unless your professor prefers two."
https://style.mla.org/number-of-spaces-after-period/

Chicago Manual of Style mandates one space after the full stop.
http://www.chicagomanualofstyle.org/qanda/data/faq/topics/OneSpaceorTwo.html

Strunk & White uses one space after the period:
https://www.legalwatercoolerblog.com/tag/strunk-and-white/

APA style (American Psychiatric Association) actually requests two
spaces after a period for *manuscripts*:
https://owl.english.purdue.edu/owl/resource/560/24/

I think it used to be the pedagogical norm to instruct students to type
two spaces after a period. Most institutions (Python documentation
excluded) do not recommend the practice currently.



*cough*



So it's not just the programming world that argues about things like
"tabs vs spaces." Literary nerds do it too.

I'm sure this email will be entirely without controversy. :)


Happy Friday,
--js
Vladimir Sementsov-Ogievskiy April 16, 2018, 11:22 a.m. UTC | #4
14.04.2018 00:06, John Snow wrote:
>
> On 04/13/2018 01:44 PM, Vladimir Sementsov-Ogievskiy wrote:
>> will add, as always, thank you for natural rewording) Hm, I have a
>> question: why do you often use double white-space "  " between
>> sentences? Is it something meaningful?
> There is some GREAT DEBATE in the English-speaking world over whether or
> not this is correct typography. At one point, it was thought that there
> should be two spaces after every full stop (".") to improve readability.
> Allegedly, this was most important for physical typesetting on typewriters.
>
> Since digital typography has taken off, some people argue that this is a
> relic and that semantically we ought to be using only one literal space
> after the full stop, and using various kerning and display parameters to
> extend the physical buffer between two sentences if desired.
>
> Famously in my mind, PEP8 mandates the two spaces after a period style.
>
> MLA says "One, unless your professor prefers two."
> https://style.mla.org/number-of-spaces-after-period/
>
> Chicago Manual of Style mandates one space after the full stop.
> http://www.chicagomanualofstyle.org/qanda/data/faq/topics/OneSpaceorTwo.html
>
> Strunk & White uses one space after the period:
> https://www.legalwatercoolerblog.com/tag/strunk-and-white/
>
> APA style (American Psychiatric Association) actually requests two
> spaces after a period for *manuscripts*:
> https://owl.english.purdue.edu/owl/resource/560/24/
>
> I think it used to be the pedagogical norm to instruct students to type
> two spaces after a period. Most institutions (Python documentation
> excluded) do not recommend the practice currently.
>
>
>
> *cough*
>
>
>
> So it's not just the programming world that argues about things like
> "tabs vs spaces." Literary nerds do it too.
>
> I'm sure this email will be entirely without controversy. :)
>
>
> Happy Friday,
> --js

Got it, thank you!
diff mbox series

Patch

diff --git a/nbd/server.c b/nbd/server.c
index b830997114..8fe53ffd4b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -733,44 +733,59 @@  static int nbd_negotiate_send_meta_context(NBDClient *client,
     return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0;
 }
 
-/* nbd_meta_base_query
- *
- * Handle query to 'base' namespace. For now, only base:allocation context is
- * available in it.  'len' is the amount of text remaining to be read from
- * the current name, after the 'base:' portion has been stripped.
+/* Read len bytes and check matching to the pattern.
+ * @match is set to true on empty query for _LIST_ and for query matching the
+ * @pattern. @match is never set to false.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
  * sending a reply about inconsistent lengths, or 1 on success. */
-static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
-                               uint32_t len, Error **errp)
+static int nbd_meta_single_query(NBDClient *client, const char *pattern,
+                                 uint32_t len, bool *match, Error **errp)
 {
     int ret;
-    char query[sizeof("allocation") - 1];
-    size_t alen = strlen("allocation");
+    char *query;
 
     if (len == 0) {
         if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
-            meta->base_allocation = true;
+            *match = true;
         }
         return 1;
     }
 
-    if (len != alen) {
+    if (len != strlen(pattern)) {
         return nbd_opt_skip(client, len, errp);
     }
 
+    query = g_malloc(len);
     ret = nbd_opt_read(client, query, len, errp);
     if (ret <= 0) {
+        g_free(query);
         return ret;
     }
 
-    if (strncmp(query, "allocation", alen) == 0) {
-        meta->base_allocation = true;
+    if (strncmp(query, pattern, len) == 0) {
+        *match = true;
     }
+    g_free(query);
 
     return 1;
 }
 
+/* nbd_meta_base_query
+ *
+ * Handle query to 'base' namespace. For now, only base:allocation context is
+ * available in it.  'len' is the amount of text remaining to be read from
+ * the current name, after the 'base:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
+                               uint32_t len, Error **errp)
+{
+    return nbd_meta_single_query(client, "allocation", len,
+                                 &meta->base_allocation, errp);
+}
+
 struct {
     const char *ns;
     int (*func)(NBDClient *, NBDExportMetaContexts *, uint32_t, Error **);