buglwIP - A Lightweight TCP/IP stack - Bugs: bug #59876, HTTPD FS: in struct fs_file,...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #59876: HTTPD FS: in struct fs_file, pextension duplicates LWIP_HTTPD_FILE_STATE functionality

Submitter:  Mike Kleshov <kleshov>
Submitted:  Fri 15 Jan 2021 06:03:56 AM UTC
   
 
Category:  apps Severity:  2 - Minor
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  kleshov
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Thu 28 Jan 2021 05:30:50 AM UTC, comment #5: 

Comitted, thank you for your feedback!

Mike Kleshov <kleshov>
Group Member
Wed 27 Jan 2021 08:16:39 PM UTC, comment #4: 

Seems good to me!

Simon Goldschmidt <goldsimon>
Group administrator
Wed 27 Jan 2021 04:58:30 PM UTC, comment #3: 

Here is the proposed change:

--- a/src/include/lwip/apps/fs.h
+++ b/src/include/lwip/apps/fs.h
@@ -67,9 +67,11 @@
 struct fs_file {
   const char *data;
   int len;
   int index;
+#if LWIP_HTTPD_FILE_EXTENSION
   /* pextension is free for implementations to hold private (extensional)
      arbitrary data, e.g. holding some file state or file system handle */
   fs_file_extension *pextension;
+#endif /* LWIP_HTTPD_FILE_EXTENSION */
 #if HTTPD_PRECALCULATED_CHECKSUM
   const struct fsdata_chksum *chksum;
   u16_t chksum_count;

--- a/src/include/lwip/apps/httpd_opts.h
+++ b/src/include/lwip/apps/httpd_opts.h
@@ -375,6 +375,16 @@
 #define LWIP_HTTPD_FILE_STATE         0
 #endif

+/** Set this to 1 to add the pextension field to the fs_file structure.
+ * This is included here to retain compatibility with legacy code that
+ * relies on the presence of the pextension field.
+ * New code should use LWIP_HTTPD_FILE_STATE instead.
+ * This option may be removed in a future version of lwip.
+ */
+#if !defined LWIP_HTTPD_FILE_EXTENSION || defined _DOXYGEN_
+#define LWIP_HTTPD_FILE_EXTENSION     0
+#endif
+
 /** HTTPD_PRECALCULATED_CHECKSUM==1: include precompiled checksums for
  * predefined (MSS-sized) chunks of the files to prevent having to calculate
  the checksums at runtime. /

Mike Kleshov <kleshov>
Group Member
Mon 25 Jan 2021 08:05:27 PM UTC, comment #2: 

Wrapping pextension in a config macro seems like the right way to go. I'll put together a patch soon so we can discuss it.

Mike Kleshov <kleshov>
Group Member
Mon 25 Jan 2021 07:32:10 PM UTC, comment #1: 

You're probably right in that it's not really required. But to ensure backwards compatibility, can we first make it conditional for some time (proabably combined with an #error that can be disabled, so that people know what to change)?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 15 Jan 2021 06:03:56 AM UTC, original submission:  

I wonder what can be done with pextension that cannot be done with LWIP_HTTPD_FILE_STATE? And one cannot disable it with a config macro. I suggest removing pextension altogether.

Mike Kleshov <kleshov>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kleshov (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-01-28 kleshov StatusNone Fixed
        Assigned toNone kleshov
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-aa77.
    Corresponding source code