Re: Examining device state via monitor for debugging (was: [PATCH 0/2] hw/misc/mos6522: Do not open-code hmp_info_human_readable_text())
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Tue, 11 Jun 2024 at 06:50, Markus Armbruster wrote: > > > > Philippe Mathieu-Daudé writes: > > > > > Officialise the QMP command, use the existing > > > hmp_info_human_readable_text() helper. > > > > I'm not sure "officialise" is a word :) > > > > Taking a step back... "info via" and its new QMP counterpart > > x-query-mos6522-devices dump device state. I understand why examining > > device state via monitor can be useful for debugging. However, we have > > more than 2000 devices in the tree. Clearly, we don't want 2000 device > > state queries. Not even 100. Could we have more generic means instead? > > > > We could use QOM (read-only) properties to expose device state. > > > > If we use one QOM property per "thing", examining device state becomes > > quite tedious. Also, you'd have to stop the guest to get a consistent > > view, and adding lots of QOM properties bloats the code. > > > > If we use a single, object-valued property for the entire state, we get > > to define the objects in QAPI. Differently tedious, and bloats the > > generated code. > > We already have a machine readable mandatory-for-every-device > representation of its entire state -- it's the vmstate struct. > Admittedly this is sometimes a bit different from the guest-facing > view of a device and we don't machine-record the field names... vmstate might not contain everything needed for debug; devices do a lot of fiddling to create a consistent vmstate, and there may be more that someone debugging wants (e.g. fd's or host memory addresses). It's also hopelessly cryptic. From an HMP point, a 'info debug ' seems good to me, possibly with some options as to whether to recurse or perhaps add flags to 'info qtree' to also call it. Dave > -- PMM -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 2/6] drm/nouveau: remove unused struct 'init_exec'
* Danilo Krummrich (d...@redhat.com) wrote:
> On 5/18/24 01:26, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'init_exec' is unused since
> > commit cb75d97e9c77 ("drm/nouveau: implement devinit subdev, and new
> > init table parser")
> > Remove it.
> >
> > Signed-off-by: Dr. David Alan Gilbert
>
> Acked-by: Danilo Krummrich
>
> To which series does this patch belong?
Actually all of them were independent patches on drm
some of which are all in, so it can be taken by itself.
> Need me to apply it?
Yes please!
Thanks,
Dave
> - Danilo
>
> > ---
> > drivers/gpu/drm/nouveau/nouveau_bios.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
> > b/drivers/gpu/drm/nouveau/nouveau_bios.c
> > index 79cfab53f80e..8c3c1f1e01c5 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> > @@ -43,11 +43,6 @@
> > #define BIOSLOG(sip, fmt, arg...) NV_DEBUG(sip->dev, fmt, ##arg)
> > #define LOG_OLD_VALUE(x)
> > -struct init_exec {
> > - bool execute;
> > - bool repeat;
> > -};
> > -
> > static bool nv_cksum(const uint8_t *data, unsigned int length)
> > {
> > /*
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/3] hw/s390x: Declare target specific monitor commands in hmp-target.h
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote: > "monitor/hmp-target.h" is meant to hold target-specific commands. > Move s390x specific commands there, slightly simplifying hmp-target.c. > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/s390x/storage-attributes.h | 4 > include/hw/s390x/storage-keys.h | 4 > include/monitor/hmp-target.h | 5 + > hw/s390x/s390-skeys.c | 2 ++ > hw/s390x/s390-stattrib.c | 2 ++ > monitor/hmp-target.c | 5 - > 6 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/include/hw/s390x/storage-attributes.h > b/include/hw/s390x/storage-attributes.h > index 8921a04d51..4916c75936 100644 > --- a/include/hw/s390x/storage-attributes.h > +++ b/include/hw/s390x/storage-attributes.h > @@ -13,7 +13,6 @@ > #define S390_STORAGE_ATTRIBUTES_H > > #include "hw/qdev-core.h" > -#include "monitor/monitor.h" > #include "qom/object.h" > > #define TYPE_S390_STATTRIB "s390-storage_attributes" > @@ -73,7 +72,4 @@ static inline Object *kvm_s390_stattrib_create(void) > } > #endif > > -void hmp_info_cmma(Monitor *mon, const QDict *qdict); > -void hmp_migrationmode(Monitor *mon, const QDict *qdict); > - > #endif /* S390_STORAGE_ATTRIBUTES_H */ > diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h > index aa2ec2aae5..1d9b7ead44 100644 > --- a/include/hw/s390x/storage-keys.h > +++ b/include/hw/s390x/storage-keys.h > @@ -13,7 +13,6 @@ > #define S390_STORAGE_KEYS_H > > #include "hw/qdev-core.h" > -#include "monitor/monitor.h" > #include "qom/object.h" > > #define TYPE_S390_SKEYS "s390-skeys" > @@ -114,7 +113,4 @@ void s390_skeys_init(void); > > S390SKeysState *s390_get_skeys_device(void); > > -void hmp_dump_skeys(Monitor *mon, const QDict *qdict); > -void hmp_info_skeys(Monitor *mon, const QDict *qdict); > - > #endif /* S390_STORAGE_KEYS_H */ > diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h > index b679aaebbf..024cff0052 100644 > --- a/include/monitor/hmp-target.h > +++ b/include/monitor/hmp-target.h > @@ -61,4 +61,9 @@ void hmp_gva2gpa(Monitor *mon, const QDict *qdict); > void hmp_gpa2hva(Monitor *mon, const QDict *qdict); > void hmp_gpa2hpa(Monitor *mon, const QDict *qdict); > > +void hmp_dump_skeys(Monitor *mon, const QDict *qdict); > +void hmp_info_skeys(Monitor *mon, const QDict *qdict); > +void hmp_info_cmma(Monitor *mon, const QDict *qdict); > +void hmp_migrationmode(Monitor *mon, const QDict *qdict); > + Could you please add a comment here saying that these are all s390, since it's not obvious from their names. (and if we're lucky the other s390 commands will stay with them). Dave > #endif /* MONITOR_HMP_TARGET_H */ > diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c > index 5c535d483e..7b2ccb94a5 100644 > --- a/hw/s390x/s390-skeys.c > +++ b/hw/s390x/s390-skeys.c > @@ -23,6 +23,8 @@ > #include "sysemu/kvm.h" > #include "migration/qemu-file-types.h" > #include "migration/register.h" > +#include "monitor/hmp-target.h" > +#include "monitor/monitor.h" > > #define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */ > #define S390_SKEYS_SAVE_FLAG_EOS 0x01 > diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c > index c4259b5327..9b4b8d8d0c 100644 > --- a/hw/s390x/s390-stattrib.c > +++ b/hw/s390x/s390-stattrib.c > @@ -19,6 +19,8 @@ > #include "exec/ram_addr.h" > #include "qapi/error.h" > #include "qapi/qmp/qdict.h" > +#include "monitor/hmp-target.h" > +#include "monitor/monitor.h" > #include "cpu.h" > > /* 512KiB cover 2GB of guest memory */ > diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c > index 1eb72ac1bf..0466474354 100644 > --- a/monitor/hmp-target.c > +++ b/monitor/hmp-target.c > @@ -36,11 +36,6 @@ > #include "qapi/error.h" > #include "qemu/cutils.h" > > -#if defined(TARGET_S390X) > -#include "hw/s390x/storage-keys.h" > -#include "hw/s390x/storage-attributes.h" > -#endif > - > /* Make devices configuration available for use in hmp-commands*.hx > templates */ > #include CONFIG_DEVICES > > -- > 2.41.0 > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH] monitor: Remove obsolete stubs
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> hmp_info_roms() was removed in commit dd98234c05 ("qapi:
> introduce x-query-roms QMP command"),
>
> hmp_info_numa() in commit 1b8ae799d8 ("qapi: introduce
> x-query-numa QMP command"),
>
> hmp_info_ramblock() in commit ca411b7c8a ("qapi: introduce
> x-query-ramblock QMP command")
>
> and hmp_info_irq() in commit 91f2fa7045 ("qapi: introduce
> x-query-irq QMP command").
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
> ---
> Note x-query-roms is used as example in
> docs/devel/writing-monitor-commands.rst
> but was removed in commit dd98234c05
> ("qapi: introduce x-query-roms QMP command").
> ---
> include/hw/loader.h | 1 -
> include/monitor/hmp.h | 3 ---
> 2 files changed, 4 deletions(-)
>
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 8685e27334..9844c5e3cf 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -338,7 +338,6 @@ void *rom_ptr(hwaddr addr, size_t size);
> * rom_ptr().
> */
> void *rom_ptr_for_as(AddressSpace *as, hwaddr addr, size_t size);
> -void hmp_info_roms(Monitor *mon, const QDict *qdict);
>
> #define rom_add_file_fixed(_f, _a, _i) \
> rom_add_file(_f, NULL, _a, _i, false, NULL, NULL)
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 954f3c83ad..ae116d9804 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -35,7 +35,6 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict);
> void hmp_info_vnc(Monitor *mon, const QDict *qdict);
> void hmp_info_spice(Monitor *mon, const QDict *qdict);
> void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> -void hmp_info_irq(Monitor *mon, const QDict *qdict);
> void hmp_info_pic(Monitor *mon, const QDict *qdict);
> void hmp_info_pci(Monitor *mon, const QDict *qdict);
> void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> @@ -102,7 +101,6 @@ void hmp_chardev_send_break(Monitor *mon, const QDict
> *qdict);
> void hmp_object_add(Monitor *mon, const QDict *qdict);
> void hmp_object_del(Monitor *mon, const QDict *qdict);
> void hmp_info_memdev(Monitor *mon, const QDict *qdict);
> -void hmp_info_numa(Monitor *mon, const QDict *qdict);
> void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
> void hmp_qom_list(Monitor *mon, const QDict *qdict);
> void hmp_qom_get(Monitor *mon, const QDict *qdict);
> @@ -141,7 +139,6 @@ void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
> void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
> void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
> void hmp_info_dump(Monitor *mon, const QDict *qdict);
> -void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
> void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
> void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
> void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
> --
> 2.41.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
* Peter Xu (pet...@redhat.com) wrote: > Hey, Dave! Hey! > On Wed, Jun 05, 2024 at 12:31:56AM +0000, Dr. David Alan Gilbert wrote: > > * Michael Galaxy (mgal...@akamai.com) wrote: > > > One thing to keep in mind here (despite me not having any hardware to > > > test) > > > was that one of the original goals here > > > in the RDMA implementation was not simply raw throughput nor raw latency, > > > but a lack of CPU utilization in kernel > > > space due to the offload. While it is entirely possible that newer > > > hardware > > > w/ TCP might compete, the significant > > > reductions in CPU usage in the TCP/IP stack were a big win at the time. > > > > > > Just something to consider while you're doing the testing > > > > I just noticed this thread; some random notes from a somewhat > > fragmented memory of this: > > > > a) Long long ago, I also tried rsocket; > > https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg02040.html > > as I remember the library was quite flaky at the time. > > Hmm interesting. There also looks like a thread doing rpoll(). Yeh, I can't actually remember much more about what I did back then! > Btw, not sure whether you noticed, but there's the series posted for the > latest rsocket conversion here: > > https://lore.kernel.org/r/1717503252-51884-1-git-send-email-arei.gong...@huawei.com Oh I hadn't; I think all of the stack of qemu's file abstractions had changed in the ~10 years since I wrote my version! > I hope Lei and his team has tested >4G mem, otherwise definitely worth > checking. Lei also mentioned there're rsocket bugs they found in the cover > letter, but not sure what's that about. It would probably be a good idea to keep track of what bugs are in flight with it, and try it on a few RDMA cards to see what problems get triggered. I think I reported a few at the time, but I gave up after feeling it was getting very hacky. > Yes, and zero-copy requires multifd for now. I think it's because we didn't > want to complicate the header processings in the migration stream where it > may not be page aligned. Ah yes. > > > > e) Someone made a good suggestion (sorry can't remember who) - that the > > RDMA migration structure was the wrong way around - it should be the > > destination which initiates an RDMA read, rather than the source > > doing a write; then things might become a LOT simpler; you just need > > to send page ranges to the destination and it can pull it. > > That might work nicely for postcopy. > > I'm not sure whether it'll still be a problem if rdma recv side is based on > zero-copy. It would be a matter of whether atomicity can be guaranteed so > that we don't want the guest vcpus to see a partially copied page during > on-flight DMAs. UFFDIO_COPY (or friend) is currently the only solution for > that. Yes, but even ignoring that (and the UFFDIO_CONTINUE idea you mention), if the destination can issue an RDMA read itself, it doesn't need to send messages to the source to ask for a page fetch; it just goes and grabs it itself, that's got to be good for latency. Dave > > Thanks, > > -- > Peter Xu > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Markus Armbruster (arm...@redhat.com) wrote: > Philippe Mathieu-Daudé writes: > > > Hi Daniel, Dave, Markus & Thomas. > > > > On 4/6/24 06:58, Markus Armbruster wrote: > >> "Dr. David Alan Gilbert" writes: > >>> * Daniel P. Berrangé (berra...@redhat.com) wrote: > >>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote: > >>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > >>>>>> We are trying to unify all qemu-system-FOO to a single binary. > >>>>>> In order to do that we need to remove QAPI target specific code. > >>>>>> > >>>>>> @dump-skeys is only available on qemu-system-s390x. This series > >>>>>> rename it as @dump-s390-skey, making it available on other > >>>>>> binaries. We take care of backward compatibility via deprecation. > >>>>>> > >>>>>> Philippe Mathieu-Daudé (4): > >>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command > >>>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command > >>>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command > >>>>>> hw/s390x: Deprecate the QMP @dump-skeys command > >>>>> > >>>>> Why do we have to rename the command? Just for the sake of it? I think > >>>>> renaming HMP commands is maybe ok, but breaking the API in QMP is > >>>>> something > >>>>> you should consider twice. > > > > I'm looking at how to include this command in the new "single binary". > > > > Markus explained in an earlier series, just expanding this command as > > stub to targets that don't implement it is not backward compatible and > > breaks QMP introspection. Currently on s390x we get a result, on other > > targets the command doesn't exist. If we add a stubs, then other targets > > return something (even if it is an empty list), confusing management > > interface. > > Loss of introspection precision is a concern, not a hard "no". > > We weigh all the concerns, and pick a solution we hate the least :) > > > So this approach use to deprecate process to include a new command > > which behaves differently on non-s390x targets. > > > > If we don't care for this particular case, better. However I'd still > > like to discuss this approach for other target-specific commands. > > > >> PRO rename: the command's tie to S390 is them immediately obvious, which > >> may be useful when the command becomes available in qemu-systems capable > >> of running other targets. > >> > >> CON rename: users need to adapt. > >> > >> What are the users? Not libvirt, as far as I can tell. > > > > Years ago we said, "all HMP must be based on QMP". > > In practice, it's closer to "HMP must be base on QMP when the > functionality does or should exist in QMP." > > >Now we realize HMP > > became stable because QMP-exposed, although not consumed externally... > > I'm afraid I didn't get this part. > > > Does the concept of "internal QMP commands" makes sense for HMP debug > > ones? (Looking at a way to not expose them). We could use the "x-" > > prefix to not care about stable / backward compat, but what is the point > > of exposing to QMP commands that will never be accessed there? > > > >>>> That was going to be my question too. Seems like its possible to simply > >>>> stub out the existing command for other targets. > >> > >> That's going to happen whether we rename the commands or not. > >> > >>> Are these commands really supposed to be stable, or are they just debug > >>> commands? If they are debug, then add the x- and don't worry too much. > > > > OK. > > > >> docs/devel/qapi-code-gen.rst: > >> > >> Names beginning with ``x-`` used to signify "experimental". This > >> convention has been replaced by special feature "unstable". > >> > >> Feature "unstable" is what makes something unstable, and is what > >> machines should check. > > > > What I mentioned earlier could be 'Feature "internal" or "debug"'. > > What's the difference to "unstable"? It should be clear *why* something is marked x- - something that's marked 'x-' because the featu
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
DPU card. > > > > > > > > > > In some scenarios where fast live migration is needed (extremely > > > > > > short > > > > > > interruption duration and migration duration) is very useful. To > > > > > > this > > > > > > end, we have also developed RDMA support for multifd. > > > > > Will any of you upstream that work? I'm curious how intrusive would > > > > > it be > > > > > when adding it to multifd, if it can keep only 5 exported functions > > > > > like what > > > > > rdma.h does right now it'll be pretty nice. We also want to make > > > > > sure it works > > > > > with arbitrary sized loads and buffers, e.g. vfio is considering to > > > > > add IO loads to > > > > > multifd channels too. > > > > > > > > > In fact, we sent the patchset to the community in 2021. Pls see: > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/20210203185906.GT2950@work-vm/T/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZl4NUEGc$ > > Yes, I have sent the patchset of multifd support for rdma migration by > > taking over my colleague, and also > > sorry for not keeping on this work at that time due to some reasons. > > And also I am strongly agree with Lei that the RDMA protocol has some > > special advantages against with TCP > > in some scenario, and we are indeed to use it in our product. > > > > > I wasn't aware of that for sure in the past.. > > > > > > Multifd has changed quite a bit in the last 9.0 release, that may not > > > apply > > > anymore. One thing to mention is please look at Dan's comment on possible > > > use of rsocket.h: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/zjjm6rcqs5eho...@redhat.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZ0CFSE-o$ > > > > > > And Jinpu did help provide an initial test result over the library: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/qemu-devel/camgffek8wiknqmouyxcathgtiem2dwocf_w7t0vmcd-i30t...@mail.gmail.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZxPNcdb4$ > > > > > > It looks like we have a chance to apply that in QEMU. > > > > > > > > > > > > One thing to note that the question here is not about a pure > > > > > performance > > > > > comparison between rdma and nics only. It's about help us make a > > > > > decision > > > > > on whether to drop rdma, iow, even if rdma performs well, the > > > > > community still > > > > > has the right to drop it if nobody can actively work and maintain it. > > > > > It's just that if nics can perform as good it's more a reason to > > > > > drop, unless > > > > > companies can help to provide good support and work together. > > > > > > > > > We are happy to provide the necessary review and maintenance work for > > > > RDMA > > > > if the community needs it. > > > > > > > > CC'ing Chuan Zheng. > > > I'm not sure whether you and Jinpu's team would like to work together and > > > provide a final solution for rdma over multifd. It could be much simpler > > > than the original 2021 proposal if the rsocket API will work out. > > > > > > Thanks, > > > > > That's a good news to see the socket abstraction for RDMA! > > When I was developed the series above, the most pain is the RDMA migration > > has no QIOChannel abstraction and i need to take a 'fake channel' > > for it which is awkward in code implementation. > > So, as far as I know, we can do this by > > i. the first thing is that we need to evaluate the rsocket is good enough > > to satisfy our QIOChannel fundamental abstraction > > ii. if it works right, then we will continue to see if it can give us > > opportunity to hide the detail of rdma protocol > > into rsocket by remove most of code in rdma.c and also some hack in > > migration main process. > > iii. implement the advanced features like multi-fd and multi-uri for rdma > > migration. > > > > Since I am not familiar with rsocket, I need some times to look at it and > > do some quick verify with rdma migration based on rsocket. > > But, yes, I am willing to involved in this refactor work and to see if we > > can make this migration feature more better:） > > > > > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH] dm cache metadata: remove unused struct 'thunk'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'thunk' has been unused since
> commit f177940a8091 ("dm cache metadata: switch to using the new
> cursor api for loading metadata").
>
> Remove it.
>
> Signed-off-by: Dr. David Alan Gilbert
Ping
> ---
> drivers/md/dm-cache-metadata.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
> index 96751cd3d181..0ad9dc1824fa 100644
> --- a/drivers/md/dm-cache-metadata.c
> +++ b/drivers/md/dm-cache-metadata.c
> @@ -1282,15 +1282,6 @@ int dm_cache_insert_mapping(struct dm_cache_metadata
> *cmd,
> return r;
> }
>
> -struct thunk {
> - load_mapping_fn fn;
> - void *context;
> -
> - struct dm_cache_metadata *cmd;
> - bool respect_dirty_flags;
> - bool hints_valid;
> -};
> -
> static bool policy_unchanged(struct dm_cache_metadata *cmd,
>struct dm_cache_policy *policy)
> {
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/7] Remove some unused structures
* Dr. David Alan Gilbert (d...@treblig.org) wrote: > A bunch of structs that are currently unused, > found with a simple script and a bit of eyeballing. > > The only one I'm that suspicious of is the SPARC > one, where the patch which removed the use is a bit > confusing to me. Copying in Trivial; I think there are 4 of these that are still outstanding: [PATCH 1/7] linux-user: cris: Remove unused struct 'rt_signal_frame' (Although cris is going) [PATCH 3/7] linux-user: sparc: Remove unused struct 'target_mc_fq' [PATCH 5/7] hw/arm/bcm2836: Remove unusued struct 'BCM283XClass' [PATCH 7/7] net/can: Remove unused struct 'CanBusState' Can Trivial pick these up please? Dave > Tested with a > > --target-list=ppc-softmmu,m68k-softmmu,x86_64-softmmu,arm-softmmu,aarch64-softmmu,cris-linux-user,i386-linux-user,sparc-linux-user > and 'make check' on x86 linux. > > Dave > > Dr. David Alan Gilbert (7): > linux-user: cris: Remove unused struct 'rt_signal_frame' > linux-user: i386/signal: Remove unused fp structs > linux-user: sparc: Remove unused struct 'target_mc_fq' > hw/usb/dev-network: Remove unused struct 'rndis_config_parameter' > hw/arm/bcm2836: Remove unusued struct 'BCM283XClass' > target/ppc: Remove unused struct 'mmu_ctx_hash32' > net/can: Remove unused struct 'CanBusState' > > hw/arm/bcm2836.c | 12 > hw/usb/dev-network.c | 8 > linux-user/cris/signal.c | 8 > linux-user/i386/signal.c | 10 -- > linux-user/sparc/signal.c | 5 - > net/can/can_host.c| 6 -- > target/ppc/mmu-hash32.c | 6 -- > 7 files changed, 55 deletions(-) > > -- > 2.45.0 > > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
* Peter Xu (pet...@redhat.com) wrote: > Hey, Dave! Hey! > On Wed, Jun 05, 2024 at 12:31:56AM +0000, Dr. David Alan Gilbert wrote: > > * Michael Galaxy (mgal...@akamai.com) wrote: > > > One thing to keep in mind here (despite me not having any hardware to > > > test) > > > was that one of the original goals here > > > in the RDMA implementation was not simply raw throughput nor raw latency, > > > but a lack of CPU utilization in kernel > > > space due to the offload. While it is entirely possible that newer > > > hardware > > > w/ TCP might compete, the significant > > > reductions in CPU usage in the TCP/IP stack were a big win at the time. > > > > > > Just something to consider while you're doing the testing > > > > I just noticed this thread; some random notes from a somewhat > > fragmented memory of this: > > > > a) Long long ago, I also tried rsocket; > > https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg02040.html > > as I remember the library was quite flaky at the time. > > Hmm interesting. There also looks like a thread doing rpoll(). Yeh, I can't actually remember much more about what I did back then! > Btw, not sure whether you noticed, but there's the series posted for the > latest rsocket conversion here: > > https://lore.kernel.org/r/1717503252-51884-1-git-send-email-arei.gong...@huawei.com Oh I hadn't; I think all of the stack of qemu's file abstractions had changed in the ~10 years since I wrote my version! > I hope Lei and his team has tested >4G mem, otherwise definitely worth > checking. Lei also mentioned there're rsocket bugs they found in the cover > letter, but not sure what's that about. It would probably be a good idea to keep track of what bugs are in flight with it, and try it on a few RDMA cards to see what problems get triggered. I think I reported a few at the time, but I gave up after feeling it was getting very hacky. > Yes, and zero-copy requires multifd for now. I think it's because we didn't > want to complicate the header processings in the migration stream where it > may not be page aligned. Ah yes. > > > > e) Someone made a good suggestion (sorry can't remember who) - that the > > RDMA migration structure was the wrong way around - it should be the > > destination which initiates an RDMA read, rather than the source > > doing a write; then things might become a LOT simpler; you just need > > to send page ranges to the destination and it can pull it. > > That might work nicely for postcopy. > > I'm not sure whether it'll still be a problem if rdma recv side is based on > zero-copy. It would be a matter of whether atomicity can be guaranteed so > that we don't want the guest vcpus to see a partially copied page during > on-flight DMAs. UFFDIO_COPY (or friend) is currently the only solution for > that. Yes, but even ignoring that (and the UFFDIO_CONTINUE idea you mention), if the destination can issue an RDMA read itself, it doesn't need to send messages to the source to ask for a page fetch; it just goes and grabs it itself, that's got to be good for latency. Dave > > Thanks, > > -- > Peter Xu > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
* Peter Xu (pet...@redhat.com) wrote: > Hey, Dave! Hey! > On Wed, Jun 05, 2024 at 12:31:56AM +0000, Dr. David Alan Gilbert wrote: > > * Michael Galaxy (mgal...@akamai.com) wrote: > > > One thing to keep in mind here (despite me not having any hardware to > > > test) > > > was that one of the original goals here > > > in the RDMA implementation was not simply raw throughput nor raw latency, > > > but a lack of CPU utilization in kernel > > > space due to the offload. While it is entirely possible that newer > > > hardware > > > w/ TCP might compete, the significant > > > reductions in CPU usage in the TCP/IP stack were a big win at the time. > > > > > > Just something to consider while you're doing the testing > > > > I just noticed this thread; some random notes from a somewhat > > fragmented memory of this: > > > > a) Long long ago, I also tried rsocket; > > https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg02040.html > > as I remember the library was quite flaky at the time. > > Hmm interesting. There also looks like a thread doing rpoll(). Yeh, I can't actually remember much more about what I did back then! > Btw, not sure whether you noticed, but there's the series posted for the > latest rsocket conversion here: > > https://lore.kernel.org/r/1717503252-51884-1-git-send-email-arei.gong...@huawei.com Oh I hadn't; I think all of the stack of qemu's file abstractions had changed in the ~10 years since I wrote my version! > I hope Lei and his team has tested >4G mem, otherwise definitely worth > checking. Lei also mentioned there're rsocket bugs they found in the cover > letter, but not sure what's that about. It would probably be a good idea to keep track of what bugs are in flight with it, and try it on a few RDMA cards to see what problems get triggered. I think I reported a few at the time, but I gave up after feeling it was getting very hacky. > Yes, and zero-copy requires multifd for now. I think it's because we didn't > want to complicate the header processings in the migration stream where it > may not be page aligned. Ah yes. > > > > e) Someone made a good suggestion (sorry can't remember who) - that the > > RDMA migration structure was the wrong way around - it should be the > > destination which initiates an RDMA read, rather than the source > > doing a write; then things might become a LOT simpler; you just need > > to send page ranges to the destination and it can pull it. > > That might work nicely for postcopy. > > I'm not sure whether it'll still be a problem if rdma recv side is based on > zero-copy. It would be a matter of whether atomicity can be guaranteed so > that we don't want the guest vcpus to see a partially copied page during > on-flight DMAs. UFFDIO_COPY (or friend) is currently the only solution for > that. Yes, but even ignoring that (and the UFFDIO_CONTINUE idea you mention), if the destination can issue an RDMA read itself, it doesn't need to send messages to the source to ask for a page fetch; it just goes and grabs it itself, that's got to be good for latency. Dave > > Thanks, > > -- > Peter Xu > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Markus Armbruster (arm...@redhat.com) wrote: > Philippe Mathieu-Daudé writes: > > > Hi Daniel, Dave, Markus & Thomas. > > > > On 4/6/24 06:58, Markus Armbruster wrote: > >> "Dr. David Alan Gilbert" writes: > >>> * Daniel P. Berrangé (berra...@redhat.com) wrote: > >>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote: > >>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > >>>>>> We are trying to unify all qemu-system-FOO to a single binary. > >>>>>> In order to do that we need to remove QAPI target specific code. > >>>>>> > >>>>>> @dump-skeys is only available on qemu-system-s390x. This series > >>>>>> rename it as @dump-s390-skey, making it available on other > >>>>>> binaries. We take care of backward compatibility via deprecation. > >>>>>> > >>>>>> Philippe Mathieu-Daudé (4): > >>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command > >>>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command > >>>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command > >>>>>> hw/s390x: Deprecate the QMP @dump-skeys command > >>>>> > >>>>> Why do we have to rename the command? Just for the sake of it? I think > >>>>> renaming HMP commands is maybe ok, but breaking the API in QMP is > >>>>> something > >>>>> you should consider twice. > > > > I'm looking at how to include this command in the new "single binary". > > > > Markus explained in an earlier series, just expanding this command as > > stub to targets that don't implement it is not backward compatible and > > breaks QMP introspection. Currently on s390x we get a result, on other > > targets the command doesn't exist. If we add a stubs, then other targets > > return something (even if it is an empty list), confusing management > > interface. > > Loss of introspection precision is a concern, not a hard "no". > > We weigh all the concerns, and pick a solution we hate the least :) > > > So this approach use to deprecate process to include a new command > > which behaves differently on non-s390x targets. > > > > If we don't care for this particular case, better. However I'd still > > like to discuss this approach for other target-specific commands. > > > >> PRO rename: the command's tie to S390 is them immediately obvious, which > >> may be useful when the command becomes available in qemu-systems capable > >> of running other targets. > >> > >> CON rename: users need to adapt. > >> > >> What are the users? Not libvirt, as far as I can tell. > > > > Years ago we said, "all HMP must be based on QMP". > > In practice, it's closer to "HMP must be base on QMP when the > functionality does or should exist in QMP." > > >Now we realize HMP > > became stable because QMP-exposed, although not consumed externally... > > I'm afraid I didn't get this part. > > > Does the concept of "internal QMP commands" makes sense for HMP debug > > ones? (Looking at a way to not expose them). We could use the "x-" > > prefix to not care about stable / backward compat, but what is the point > > of exposing to QMP commands that will never be accessed there? > > > >>>> That was going to be my question too. Seems like its possible to simply > >>>> stub out the existing command for other targets. > >> > >> That's going to happen whether we rename the commands or not. > >> > >>> Are these commands really supposed to be stable, or are they just debug > >>> commands? If they are debug, then add the x- and don't worry too much. > > > > OK. > > > >> docs/devel/qapi-code-gen.rst: > >> > >> Names beginning with ``x-`` used to signify "experimental". This > >> convention has been replaced by special feature "unstable". > >> > >> Feature "unstable" is what makes something unstable, and is what > >> machines should check. > > > > What I mentioned earlier could be 'Feature "internal" or "debug"'. > > What's the difference to "unstable"? It should be clear *why* something is marked x- - something that's marked 'x-' because the featu
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
DPU card. > > > > > > > > > > In some scenarios where fast live migration is needed (extremely > > > > > > short > > > > > > interruption duration and migration duration) is very useful. To > > > > > > this > > > > > > end, we have also developed RDMA support for multifd. > > > > > Will any of you upstream that work? I'm curious how intrusive would > > > > > it be > > > > > when adding it to multifd, if it can keep only 5 exported functions > > > > > like what > > > > > rdma.h does right now it'll be pretty nice. We also want to make > > > > > sure it works > > > > > with arbitrary sized loads and buffers, e.g. vfio is considering to > > > > > add IO loads to > > > > > multifd channels too. > > > > > > > > > In fact, we sent the patchset to the community in 2021. Pls see: > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/20210203185906.GT2950@work-vm/T/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZl4NUEGc$ > > Yes, I have sent the patchset of multifd support for rdma migration by > > taking over my colleague, and also > > sorry for not keeping on this work at that time due to some reasons. > > And also I am strongly agree with Lei that the RDMA protocol has some > > special advantages against with TCP > > in some scenario, and we are indeed to use it in our product. > > > > > I wasn't aware of that for sure in the past.. > > > > > > Multifd has changed quite a bit in the last 9.0 release, that may not > > > apply > > > anymore. One thing to mention is please look at Dan's comment on possible > > > use of rsocket.h: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/zjjm6rcqs5eho...@redhat.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZ0CFSE-o$ > > > > > > And Jinpu did help provide an initial test result over the library: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/qemu-devel/camgffek8wiknqmouyxcathgtiem2dwocf_w7t0vmcd-i30t...@mail.gmail.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZxPNcdb4$ > > > > > > It looks like we have a chance to apply that in QEMU. > > > > > > > > > > > > One thing to note that the question here is not about a pure > > > > > performance > > > > > comparison between rdma and nics only. It's about help us make a > > > > > decision > > > > > on whether to drop rdma, iow, even if rdma performs well, the > > > > > community still > > > > > has the right to drop it if nobody can actively work and maintain it. > > > > > It's just that if nics can perform as good it's more a reason to > > > > > drop, unless > > > > > companies can help to provide good support and work together. > > > > > > > > > We are happy to provide the necessary review and maintenance work for > > > > RDMA > > > > if the community needs it. > > > > > > > > CC'ing Chuan Zheng. > > > I'm not sure whether you and Jinpu's team would like to work together and > > > provide a final solution for rdma over multifd. It could be much simpler > > > than the original 2021 proposal if the rsocket API will work out. > > > > > > Thanks, > > > > > That's a good news to see the socket abstraction for RDMA! > > When I was developed the series above, the most pain is the RDMA migration > > has no QIOChannel abstraction and i need to take a 'fake channel' > > for it which is awkward in code implementation. > > So, as far as I know, we can do this by > > i. the first thing is that we need to evaluate the rsocket is good enough > > to satisfy our QIOChannel fundamental abstraction > > ii. if it works right, then we will continue to see if it can give us > > opportunity to hide the detail of rdma protocol > > into rsocket by remove most of code in rdma.c and also some hack in > > migration main process. > > iii. implement the advanced features like multi-fd and multi-uri for rdma > > migration. > > > > Since I am not familiar with rsocket, I need some times to look at it and > > do some quick verify with rdma migration based on rsocket. > > But, yes, I am willing to involved in this refactor work and to see if we > > can make this migration feature more better:） > > > > > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
DPU card. > > > > > > > > > > In some scenarios where fast live migration is needed (extremely > > > > > > short > > > > > > interruption duration and migration duration) is very useful. To > > > > > > this > > > > > > end, we have also developed RDMA support for multifd. > > > > > Will any of you upstream that work? I'm curious how intrusive would > > > > > it be > > > > > when adding it to multifd, if it can keep only 5 exported functions > > > > > like what > > > > > rdma.h does right now it'll be pretty nice. We also want to make > > > > > sure it works > > > > > with arbitrary sized loads and buffers, e.g. vfio is considering to > > > > > add IO loads to > > > > > multifd channels too. > > > > > > > > > In fact, we sent the patchset to the community in 2021. Pls see: > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/20210203185906.GT2950@work-vm/T/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZl4NUEGc$ > > Yes, I have sent the patchset of multifd support for rdma migration by > > taking over my colleague, and also > > sorry for not keeping on this work at that time due to some reasons. > > And also I am strongly agree with Lei that the RDMA protocol has some > > special advantages against with TCP > > in some scenario, and we are indeed to use it in our product. > > > > > I wasn't aware of that for sure in the past.. > > > > > > Multifd has changed quite a bit in the last 9.0 release, that may not > > > apply > > > anymore. One thing to mention is please look at Dan's comment on possible > > > use of rsocket.h: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/zjjm6rcqs5eho...@redhat.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZ0CFSE-o$ > > > > > > And Jinpu did help provide an initial test result over the library: > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/qemu-devel/camgffek8wiknqmouyxcathgtiem2dwocf_w7t0vmcd-i30t...@mail.gmail.com/__;!!GjvTz_vk!VfP_SV-8uRya7rBdopv8OUJkmnSi44Ktpqq1E7sr_Xcwt6zvveW51qboWOBSTChdUG1hJwfAl7HZxPNcdb4$ > > > > > > It looks like we have a chance to apply that in QEMU. > > > > > > > > > > > > One thing to note that the question here is not about a pure > > > > > performance > > > > > comparison between rdma and nics only. It's about help us make a > > > > > decision > > > > > on whether to drop rdma, iow, even if rdma performs well, the > > > > > community still > > > > > has the right to drop it if nobody can actively work and maintain it. > > > > > It's just that if nics can perform as good it's more a reason to > > > > > drop, unless > > > > > companies can help to provide good support and work together. > > > > > > > > > We are happy to provide the necessary review and maintenance work for > > > > RDMA > > > > if the community needs it. > > > > > > > > CC'ing Chuan Zheng. > > > I'm not sure whether you and Jinpu's team would like to work together and > > > provide a final solution for rdma over multifd. It could be much simpler > > > than the original 2021 proposal if the rsocket API will work out. > > > > > > Thanks, > > > > > That's a good news to see the socket abstraction for RDMA! > > When I was developed the series above, the most pain is the RDMA migration > > has no QIOChannel abstraction and i need to take a 'fake channel' > > for it which is awkward in code implementation. > > So, as far as I know, we can do this by > > i. the first thing is that we need to evaluate the rsocket is good enough > > to satisfy our QIOChannel fundamental abstraction > > ii. if it works right, then we will continue to see if it can give us > > opportunity to hide the detail of rdma protocol > > into rsocket by remove most of code in rdma.c and also some hack in > > migration main process. > > iii. implement the advanced features like multi-fd and multi-uri for rdma > > migration. > > > > Since I am not familiar with rsocket, I need some times to look at it and > > do some quick verify with rdma migration based on rsocket. > > But, yes, I am willing to involved in this refactor work and to see if we > > can make this migration feature more better:） > > > > > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH v2 06/18] monitor: Stop removing non-duplicated fds
; QLIST_REMOVE(mon_fdset_fd_dup, next);
> >> g_free(mon_fdset_fd_dup);
> >> -if (QLIST_EMPTY(_fdset->dup_fds)) {
> >> -monitor_fdset_cleanup(mon_fdset);
> >> -}
> >> +monitor_fdset_free(mon_fdset);
> >
> > This and above changes are not crystal clear to me where the _cleanup()
> > does extra check "removed" and clean those fds.
> >
> > I think it'll make it easier for me to understand if this one and the next
> > are squashed together. But maybe it's simply because I didn't fully
> > understand.
>
> monitor_fdsets_cleanup() was doing three things previously:
>
> 1- Remove the removed=true fds. This is weird, but ok.
>
> 2- Remove fds from an fdset that has an empty dup_fds list, but only if
> the guest is not running and the monitor is closed. This is problematic.
>
> 3- Remove/free fdsets that have become empty due to the above
> removals. This is ok.
>
> This patch covers (2), because that is the only change that has a
> complex reasoning behind it and we need to document that without
> conflating it with the rest of the changes.
The ebe52b592d you reference, makes me think that the only reason for the
'is not running' was as a way to detect when init had finished; and there
are certainly better ways to do that (now?).
However, the efb87c1697 talks about cleaning up non-dup's on all monitors
closed, to stop getting left-over fd's that were added but then not used
(because a disconnect happened between adding and being used).
In your world when do these get cleaned up?
Dave
> Since (3) is still a reasonable thing to do, this patch merely keeps it
> around, but using the helpers introduced in the previous patch.
>
> The next patch removed the need for (1), making the removal immediate
> instead of delayed. It has it's own much less complex reasoning, which
> is: "we don't need to wait to remove the fd".
>
> I hope this clarifies a bit. I would prefer if we kept this and the next
> patch separate to avoid having a single patch that does too many
> things. I hope to be as explicit as possible with the reason behind
> these changes to avoid putting future people in the situation that we
> are in now, i.e. having to guess at the reasons behind this code.
>
> If you still think we should squash or if you have more questions, let
> me know.
>
> >> return;
> >> }
> >> }
> >> diff --git a/monitor/hmp.c b/monitor/hmp.c
> >> index 69c1b7e98a..460e8832f6 100644
> >> --- a/monitor/hmp.c
> >> +++ b/monitor/hmp.c
> >> @@ -1437,11 +1437,9 @@ static void monitor_event(void *opaque,
> >> QEMUChrEvent event)
> >> monitor_resume(mon);
> >> }
> >> qemu_mutex_unlock(>mon_lock);
> >> -mon_refcount++;
> >> break;
> >>
> >> case CHR_EVENT_CLOSED:
> >> -mon_refcount--;
> >> monitor_fdsets_cleanup();
> >> break;
> >>
> >> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> >> index 252de85681..cb628f681d 100644
> >> --- a/monitor/monitor-internal.h
> >> +++ b/monitor/monitor-internal.h
> >> @@ -168,7 +168,6 @@ extern bool qmp_dispatcher_co_shutdown;
> >> extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
> >> extern QemuMutex monitor_lock;
> >> extern MonitorList mon_list;
> >> -extern int mon_refcount;
> >>
> >> extern HMPCommand hmp_cmds[];
> >>
> >> diff --git a/monitor/qmp.c b/monitor/qmp.c
> >> index a239945e8d..5e538f34c0 100644
> >> --- a/monitor/qmp.c
> >> +++ b/monitor/qmp.c
> >> @@ -466,7 +466,6 @@ static void monitor_qmp_event(void *opaque,
> >> QEMUChrEvent event)
> >> data = qmp_greeting(mon);
> >> qmp_send_response(mon, data);
> >> qobject_unref(data);
> >> -mon_refcount++;
> >> break;
> >> case CHR_EVENT_CLOSED:
> >> /*
> >> @@ -479,7 +478,6 @@ static void monitor_qmp_event(void *opaque,
> >> QEMUChrEvent event)
> >> json_message_parser_destroy(>parser);
> >> json_message_parser_init(>parser, handle_qmp_command,
> >> mon, NULL);
> >> -mon_refcount--;
> >> monitor_fdsets_cleanup();
> >> break;
> >> case CHR_EVENT_BREAK:
> >
> > I like this too when mon_refcount can be dropped. It turns out I like this
> > patch and the next a lot, and I hope nothing will break.
> >
> > Aside, you seem to have forgot removal of the "int mon_refcount" in
> > monitor.c.
>
> Yes, I'll fix that. Thanks.
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote: > > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > > > We are trying to unify all qemu-system-FOO to a single binary. > > > In order to do that we need to remove QAPI target specific code. > > > > > > @dump-skeys is only available on qemu-system-s390x. This series > > > rename it as @dump-s390-skey, making it available on other > > > binaries. We take care of backward compatibility via deprecation. > > > > > > Philippe Mathieu-Daudé (4): > > >hw/s390x: Introduce the @dump-s390-skeys QMP command > > >hw/s390x: Introduce the 'dump_s390_skeys' HMP command > > >hw/s390x: Deprecate the HMP 'dump_skeys' command > > >hw/s390x: Deprecate the QMP @dump-skeys command > > > > Why do we have to rename the command? Just for the sake of it? I think > > renaming HMP commands is maybe ok, but breaking the API in QMP is something > > you should consider twice. > > That was going to be my question too. Seems like its possible to simply > stub out the existing command for other targets. Are these commands really supposed to be stable, or are they just debug commands? If they are debug, then add the x- and don't worry too much. Dave > The renaming is just window dressing. > > > > > And even if we decide to rename ... maybe we should discuss whether it makes > > sense to come up with a generic command instead: As far as I know, ARM also > > has something similar, called MTE. Maybe we also want to dump MTE keys one > > day? So the new command should maybe be called "dump-memory-keys" instead? > > Or should it maybe rather be an option to the existing "dump-guest-memory" > > command instead? > > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Thomas Huth (th...@redhat.com) wrote:
> On 31/05/2024 16.02, Dr. David Alan Gilbert wrote:
> > * Thomas Huth (th...@redhat.com) wrote:
> > > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > > > We are trying to unify all qemu-system-FOO to a single binary.
> > > > In order to do that we need to remove QAPI target specific code.
> > > >
> > > > @dump-skeys is only available on qemu-system-s390x. This series
> > > > rename it as @dump-s390-skey, making it available on other
> > > > binaries. We take care of backward compatibility via deprecation.
> > > >
> > > > Philippe Mathieu-Daudé (4):
> > > > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > > > hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> > > > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > > > hw/s390x: Deprecate the QMP @dump-skeys command
> > >
> > > Why do we have to rename the command? Just for the sake of it? I think
> > > renaming HMP commands is maybe ok, but breaking the API in QMP is
> > > something
> > > you should consider twice.
> > >
> > > And even if we decide to rename ... maybe we should discuss whether it
> > > makes
> > > sense to come up with a generic command instead: As far as I know, ARM
> > > also
> > > has something similar, called MTE. Maybe we also want to dump MTE keys one
> > > day? So the new command should maybe be called "dump-memory-keys" instead?
> >
> > I think there are at least two different concepts; but I agree it would be
> > nice to keep a single command for matching concepts across different
> > architectures;
> > I can't say I know the details of any, but:
> >
> >a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing
> > where pages marked a special way are associated with keys.
> > That sounds similar to what the skeys are???
>
> Sounds a little bit similar, but s390 storage keys are independent from page
> tables. It's rather that each page (4096 bytes) of RAM has an additional
> 7-bit value that contains the storage key and some additional bits. It's
> also usable when page tables are still disabled.
>
> > I'm not sure the two fit in the same command.
>
> Does it make sense to dump all the MTE or x86 keys all at once? If so, we
> could maybe come up with an unified command. Otherwise it might not make
> sense, indeed.
So they're all really different granularities:
s390 - one key/physical page
ARM MTE - one tag/16 bytes physical
x86 - per virtual page; then a current register with 16 permission
sets
For x86 I guess it would be easy to dump the register, and then the
values for a range of virtual addresses.
But then if you dumped a range of physical addresses on ARM MTE you'd
get hundreds times more output than for the equivalent s390.
Dave
> Thomas
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote: > > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > > > We are trying to unify all qemu-system-FOO to a single binary. > > > In order to do that we need to remove QAPI target specific code. > > > > > > @dump-skeys is only available on qemu-system-s390x. This series > > > rename it as @dump-s390-skey, making it available on other > > > binaries. We take care of backward compatibility via deprecation. > > > > > > Philippe Mathieu-Daudé (4): > > >hw/s390x: Introduce the @dump-s390-skeys QMP command > > >hw/s390x: Introduce the 'dump_s390_skeys' HMP command > > >hw/s390x: Deprecate the HMP 'dump_skeys' command > > >hw/s390x: Deprecate the QMP @dump-skeys command > > > > Why do we have to rename the command? Just for the sake of it? I think > > renaming HMP commands is maybe ok, but breaking the API in QMP is something > > you should consider twice. > > That was going to be my question too. Seems like its possible to simply > stub out the existing command for other targets. Are these commands really supposed to be stable, or are they just debug commands? If they are debug, then add the x- and don't worry too much. Dave > The renaming is just window dressing. > > > > > And even if we decide to rename ... maybe we should discuss whether it makes > > sense to come up with a generic command instead: As far as I know, ARM also > > has something similar, called MTE. Maybe we also want to dump MTE keys one > > day? So the new command should maybe be called "dump-memory-keys" instead? > > Or should it maybe rather be an option to the existing "dump-guest-memory" > > command instead? > > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 2/6] drm/nouveau: remove unused struct 'init_exec'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'init_exec' is unused since
> commit cb75d97e9c77 ("drm/nouveau: implement devinit subdev, and new
> init table parser")
> Remove it.
>
> Signed-off-by: Dr. David Alan Gilbert
Ping.
> ---
> drivers/gpu/drm/nouveau/nouveau_bios.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
> b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index 79cfab53f80e..8c3c1f1e01c5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -43,11 +43,6 @@
> #define BIOSLOG(sip, fmt, arg...) NV_DEBUG(sip->dev, fmt, ##arg)
> #define LOG_OLD_VALUE(x)
>
> -struct init_exec {
> - bool execute;
> - bool repeat;
> -};
> -
> static bool nv_cksum(const uint8_t *data, unsigned int length)
> {
> /*
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 3/6] drm/vmwgfx: remove unused struct 'vmw_stdu_dma'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'vmw_stdu_dma' is unused since
> commit 39985eea5a6d ("drm/vmwgfx: Abstract placement selection")
> Remove it.
Ping.
> Signed-off-by: Dr. David Alan Gilbert
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 2041c4d48daa..50022e9e3519 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -85,11 +85,6 @@ struct vmw_stdu_update {
> SVGA3dCmdUpdateGBScreenTarget body;
> };
>
> -struct vmw_stdu_dma {
> - SVGA3dCmdHeader header;
> - SVGA3dCmdSurfaceDMA body;
> -};
> -
> struct vmw_stdu_surface_copy {
> SVGA3dCmdHeader header;
> SVGA3dCmdSurfaceCopy body;
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Thomas Huth (th...@redhat.com) wrote:
> On 31/05/2024 16.02, Dr. David Alan Gilbert wrote:
> > * Thomas Huth (th...@redhat.com) wrote:
> > > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > > > We are trying to unify all qemu-system-FOO to a single binary.
> > > > In order to do that we need to remove QAPI target specific code.
> > > >
> > > > @dump-skeys is only available on qemu-system-s390x. This series
> > > > rename it as @dump-s390-skey, making it available on other
> > > > binaries. We take care of backward compatibility via deprecation.
> > > >
> > > > Philippe Mathieu-Daudé (4):
> > > > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > > > hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> > > > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > > > hw/s390x: Deprecate the QMP @dump-skeys command
> > >
> > > Why do we have to rename the command? Just for the sake of it? I think
> > > renaming HMP commands is maybe ok, but breaking the API in QMP is
> > > something
> > > you should consider twice.
> > >
> > > And even if we decide to rename ... maybe we should discuss whether it
> > > makes
> > > sense to come up with a generic command instead: As far as I know, ARM
> > > also
> > > has something similar, called MTE. Maybe we also want to dump MTE keys one
> > > day? So the new command should maybe be called "dump-memory-keys" instead?
> >
> > I think there are at least two different concepts; but I agree it would be
> > nice to keep a single command for matching concepts across different
> > architectures;
> > I can't say I know the details of any, but:
> >
> >a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing
> > where pages marked a special way are associated with keys.
> > That sounds similar to what the skeys are???
>
> Sounds a little bit similar, but s390 storage keys are independent from page
> tables. It's rather that each page (4096 bytes) of RAM has an additional
> 7-bit value that contains the storage key and some additional bits. It's
> also usable when page tables are still disabled.
>
> > I'm not sure the two fit in the same command.
>
> Does it make sense to dump all the MTE or x86 keys all at once? If so, we
> could maybe come up with an unified command. Otherwise it might not make
> sense, indeed.
So they're all really different granularities:
s390 - one key/physical page
ARM MTE - one tag/16 bytes physical
x86 - per virtual page; then a current register with 16 permission
sets
For x86 I guess it would be easy to dump the register, and then the
values for a range of virtual addresses.
But then if you dumped a range of physical addresses on ARM MTE you'd
get hundreds times more output than for the equivalent s390.
Dave
> Thomas
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] list: test: remove unused struct 'klist_test_struct'
* Muhammad Usama Anjum (usama.an...@collabora.com) wrote:
> On 5/31/24 8:18 PM, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'klist_test_struct' has been unused since the original
> > commit 57b4f760f94d ("list: test: Test the klist structure").
> Probably a fixes by tag would be needed here.
I'm generally avoiding fixes tags in this set of changes, since
a) They have no behavioural change at all.
b) Downstream and stable kernel people use fixes tags to indicate
stuff they should pick up if they have the original, and there's
no need for them to do that with this cleanup.
Dave
> >
> > Remove it.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > lib/list-test.c | 6 --
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/lib/list-test.c b/lib/list-test.c
> > index 0cc27de9cec8..383ee0ad582e 100644
> > --- a/lib/list-test.c
> > +++ b/lib/list-test.c
> > @@ -1201,12 +1201,6 @@ static struct kunit_suite hlist_test_module = {
> > };
> >
> >
> > -struct klist_test_struct {
> > - int data;
> > - struct klist klist;
> > - struct klist_node klist_node;
> > -};
> > -
> > static int node_count;
> > static struct klist_node *last_node;
> >
>
> --
> BR,
> Muhammad Usama Anjum
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Thomas Huth (th...@redhat.com) wrote: > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > > We are trying to unify all qemu-system-FOO to a single binary. > > In order to do that we need to remove QAPI target specific code. > > > > @dump-skeys is only available on qemu-system-s390x. This series > > rename it as @dump-s390-skey, making it available on other > > binaries. We take care of backward compatibility via deprecation. > > > > Philippe Mathieu-Daudé (4): > >hw/s390x: Introduce the @dump-s390-skeys QMP command > >hw/s390x: Introduce the 'dump_s390_skeys' HMP command > >hw/s390x: Deprecate the HMP 'dump_skeys' command > >hw/s390x: Deprecate the QMP @dump-skeys command > > Why do we have to rename the command? Just for the sake of it? I think > renaming HMP commands is maybe ok, but breaking the API in QMP is something > you should consider twice. > > And even if we decide to rename ... maybe we should discuss whether it makes > sense to come up with a generic command instead: As far as I know, ARM also > has something similar, called MTE. Maybe we also want to dump MTE keys one > day? So the new command should maybe be called "dump-memory-keys" instead? I think there are at least two different concepts; but I agree it would be nice to keep a single command for matching concepts across different architectures; I can't say I know the details of any, but: a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing where pages marked a special way are associated with keys. That sounds similar to what the skeys are??? b) Upper bit things - where you steal a few bits from the virtual address and then use that to associate some security; I think that's closer to what MTE is isn't it? I'm not sure the two fit in the same command. Dave > Or should it maybe rather be an option to the existing "dump-guest-memory" > command instead? > > Thomas > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH v2 2/6] Convert 'info tlb' to use generic iterator
> -}
> -}
> -}
> -}
> -}
> -}
> +/* Trim line to fit screen */
> +if (pos - buf > 79) {
> +strcpy(buf + 77, "..");
> }
> -}
>
> -#ifdef TARGET_X86_64
> -static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> -uint64_t l0, uint64_t pml4_addr)
> -{
> -uint64_t l1, l2, l3, l4;
> -uint64_t pml4e, pdpe, pde, pte;
> -uint64_t pdp_addr, pd_addr, pt_addr;
> -
> -for (l1 = 0; l1 < 512; l1++) {
> -cpu_physical_memory_read(pml4_addr + l1 * 8, , 8);
> -pml4e = le64_to_cpu(pml4e);
> -if (!(pml4e & PG_PRESENT_MASK)) {
> -continue;
> -}
> -
> -pdp_addr = pml4e & 0x3f000ULL;
> -for (l2 = 0; l2 < 512; l2++) {
> -cpu_physical_memory_read(pdp_addr + l2 * 8, , 8);
> -pdpe = le64_to_cpu(pdpe);
> -if (!(pdpe & PG_PRESENT_MASK)) {
> -continue;
> -}
> -
> -if (pdpe & PG_PSE_MASK) {
> -/* 1G pages, CR4.PSE is ignored */
> -print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30),
> -pdpe, 0x3c000ULL);
> -continue;
> -}
> -
> -pd_addr = pdpe & 0x3f000ULL;
> -for (l3 = 0; l3 < 512; l3++) {
> -cpu_physical_memory_read(pd_addr + l3 * 8, , 8);
> -pde = le64_to_cpu(pde);
> -if (!(pde & PG_PRESENT_MASK)) {
> -continue;
> -}
> -
> -if (pde & PG_PSE_MASK) {
> -/* 2M pages, CR4.PSE is ignored */
> -print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30)
> +
> -(l3 << 21), pde, 0x3ffe0ULL);
> -continue;
> -}
> -
> -pt_addr = pde & 0x3f000ULL;
> -for (l4 = 0; l4 < 512; l4++) {
> -cpu_physical_memory_read(pt_addr
> -+ l4 * 8,
> -, 8);
> -pte = le64_to_cpu(pte);
> -if (pte & PG_PRESENT_MASK) {
> -print_pte(mon, env, (l0 << 48) + (l1 << 39) +
> -(l2 << 30) + (l3 << 21) + (l4 << 12),
> -pte & ~PG_PSE_MASK, 0x3f000ULL);
> -}
> -}
> -}
> -}
> -}
> +monitor_printf(mon, "%s\n", buf);
> }
>
> -static void tlb_info_la57(Monitor *mon, CPUArchState *env)
> +static
> +int mem_print_tlb(CPUState *cs, void *data, PTE_t *pte,
> + target_ulong vaddr, int height, int offset)
> {
> -uint64_t l0;
> -uint64_t pml5e;
> -uint64_t pml5_addr;
> -
> -pml5_addr = env->cr[3] & 0x3f000ULL;
> -for (l0 = 0; l0 < 512; l0++) {
> -cpu_physical_memory_read(pml5_addr + l0 * 8, , 8);
> -pml5e = le64_to_cpu(pml5e);
> -if (pml5e & PG_PRESENT_MASK) {
> -tlb_info_la48(mon, env, l0, pml5e & 0x3f000ULL);
> -}
> -}
> +struct mem_print_state *state = (struct mem_print_state *) data;
> +print_pte(state->mon, state->env, vaddr, pte->pte64_t);
> +return 0;
> }
> -#endif /* TARGET_X86_64 */
>
> void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> {
> -CPUArchState *env;
> +struct mem_print_state state;
>
> -env = mon_get_cpu_env(mon);
> -if (!env) {
> -monitor_printf(mon, "No CPU available\n");
> +CPUState *cs = mon_get_cpu(mon);
> +if (!cs) {
> +monitor_printf(mon, "Unable to get CPUState. Internal error\n");
> return;
> }
>
> -if (!(env->cr[0] & CR0_PG_MASK)) {
> -monitor_printf(mon, "PG disabled\n");
> +if (!init_iterator(mon, )) {
> return;
> }
> -if (env->cr[4] & CR4_PAE_MASK) {
> -#ifdef TARGET_X86_64
> -if (env->hflags & HF_LMA_MASK) {
> -if (env->cr[4] & CR4_LA57_MASK) {
> -tlb_info_la57(mon, env);
> -} else {
> -tlb_info_la48(mon, env, 0, env->cr[3] & 0x3f000ULL);
> -}
> -} else
> -#endif
> -{
> -tlb_info_pae32(mon, env);
> -}
> -} else {
> -tlb_info_32(mon, env);
> -}
> +
> +/**
> + * 'info tlb' visits only leaf PTEs marked present.
> + * It does not check other protection bits.
> + */
> +for_each_pte(cs, _print_tlb, , false, false);
> }
>
> static void mem_print(Monitor *mon, CPUArchState *env,
> --
> 2.34.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH v2 1/6] Add an "info pg" command that prints the current page tables
+}
> +
> +/* Trim line to fit screen */
> +if (pos - buf > 79) {
> +strcpy(buf + 77, "..");
> +}
> +
> +monitor_printf(mon, "%s\n", buf);
> +}
> +
> +static inline
> +int ent2prot(uint64_t prot)
> +{
> +return prot & (PG_USER_MASK | PG_RW_MASK |
> + PG_PRESENT_MASK);
> +}
> +
> +/* Returns true if it emitted anything */
> +static
> +bool flush_print_pg_state(CPUState *cs, struct mem_print_state *state)
> +{
> +bool ret = false;
> +for (int i = state->start_height; i > 0; i--) {
> +if (state->vstart[i] == -1) {
> +break;
> +}
> +PTE_t my_pte;
> +my_pte.pte64_t = state->ent[i];
> +ret = true;
> +pg_print(cs, state->mon, state->ent[i],
> + state->vstart[i], state->vend[i],
> + state->pstart, state->pend,
> + state->offset[i], state->last_offset[i],
> + i, state->max_height, state->vaw, state->paw,
> + mmu_pte_leaf(cs, i, _pte));
> +}
> +
> +return ret;
> +}
> +
> /* Perform linear address sign extension */
> static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
> {
> @@ -49,6 +244,191 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr
> addr)
> return addr;
> }
>
> +
> +
> +/*** Start generic page table monitor code */
> +
> +/* Assume only called on present entries */
> +static
> +int compressing_iterator(CPUState *cs, void *data, PTE_t *pte,
> + target_ulong vaddr, int height, int offset)
> +{
> +struct mem_print_state *state = (struct mem_print_state *) data;
> +hwaddr paddr = mmu_pte_child(cs, pte, height);
> +target_ulong size = mmu_pte_leaf_page_size(cs, height);
> +bool start_new_run = false, flush = false;
> +bool is_leaf = mmu_pte_leaf(cs, height, pte);
> +
> +int entries_per_node = mmu_page_table_entries_per_node(cs, height);
> +
> +/* Prot of current pte */
> +int prot = ent2prot(pte->pte64_t);
> +
> +
> +/* If there is a prior run, first try to extend it. */
> +if (state->start_height != 0) {
> +
> +/*
> + * If we aren't flushing interior nodes, raise the start height.
> + * We don't need to detect non-compressible interior nodes.
> + */
> +if ((!state->flush_interior) && state->start_height < height) {
> +state->start_height = height;
> +state->vstart[height] = vaddr;
> +state->vend[height] = vaddr;
> +state->ent[height] = pte->pte64_t;
> +if (offset == 0) {
> +state->last_offset[height] = entries_per_node - 1;
> +} else {
> +state->last_offset[height] = offset - 1;
> +}
> +}
> +
> +/* Detect when we are walking down the "left edge" of a range */
> +if (state->vstart[height] == -1
> +&& (height + 1) <= state->start_height
> +&& state->vstart[height + 1] == vaddr) {
> +
> +state->vstart[height] = vaddr;
> +state->vend[height] = vaddr;
> +state->ent[height] = pte->pte64_t;
> +state->offset[height] = offset;
> +state->last_offset[height] = offset;
> +
> +if (is_leaf) {
> +state->pstart = paddr;
> +state->pend = paddr;
> +}
> +
> +/* Detect contiguous entries at same level */
> +} else if ((state->vstart[height] != -1)
> + && (state->start_height >= height)
> + && ent2prot(state->ent[height]) == prot
> + && (((state->last_offset[height] + 1) % entries_per_node)
> + == offset)
> + && ((!is_leaf)
> + || (!state->require_physical_contiguity)
> + || (state->pend + size == paddr))) {
> +
> +
> +/*
> + * If there are entries at the levels below, make sure we
> + * completed them. We only compress interior nodes
> + * without holes in the mappings.
> + */
> +if (height != 1) {
> +for (int i = height - 1; i >= 1; i--) {
> +int entries = mmu_page_table_entries_per_node(cs, i);
> +
> +/* Stop if we hit large pages before level 1 */
> +if (state->vstart[i] == -1) {
> +break;
> +}
> +
> +if ((state->last_offset[i] + 1) != entries) {
> +flush = true;
> +start_new_run = true;
> +break;
> +}
> +}
> +}
> +
> +
> +if (!flush) {
> +
> +/* We can compress these entries */
> +state->ent[height] = pte->pte64_t;
> +state->vend[height] = vaddr;
> +state->last_offset[height] = offset;
> +
> +/* Only update the physical range on leaves */
> +if (is_leaf) {
> +state->pend = paddr;
> +}
> +}
> +/* Let PTEs accumulate... */
> +} else {
> +flush = true;
> +}
> +
> +if (flush) {
> +/*
> + * We hit dicontiguous permissions or pages.
> + * Print the old entries, then start accumulating again
> + *
> + * Some clients only want the flusher called on a leaf.
> + * Check that too.
> + *
> + * We can infer whether the accumulated range includes a
> + * leaf based on whether pstart is -1.
> + */
> +if (state->flush_interior || (state->pstart != -1)) {
> +if (state->flusher(cs, state)) {
> +start_new_run = true;
> +}
> +} else {
> +start_new_run = true;
> +}
> +}
> +} else {
> +start_new_run = true;
> +}
> +
> +if (start_new_run) {
> +/* start a new run with this PTE */
> +for (int i = state->start_height; i > 0; i--) {
> +if (state->vstart[i] != -1) {
> +state->ent[i] = 0;
> +state->last_offset[i] = 0;
> +state->vstart[i] = -1;
> +}
> +}
> +state->pstart = -1;
> +state->vstart[height] = vaddr;
> +state->vend[height] = vaddr;
> +state->ent[height] = pte->pte64_t;
> +state->offset[height] = offset;
> +state->last_offset[height] = offset;
> +if (is_leaf) {
> +state->pstart = paddr;
> +state->pend = paddr;
> +}
> +state->start_height = height;
> +}
> +
> +return 0;
> +}
> +
> +
> +void hmp_info_pg(Monitor *mon, const QDict *qdict)
> +{
> +struct mem_print_state state;
> +
> +CPUState *cs = mon_get_cpu(mon);
> +if (!cs) {
> +monitor_printf(mon, "Unable to get CPUState. Internal error\n");
> +return;
> +}
> +
> +if (!init_iterator(mon, )) {
> +return;
> +}
> +state.flush_interior = true;
> +state.require_physical_contiguity = true;
> +state.flusher = _print_pg_state;
> +
> +pg_print_header(mon, );
> +
> +/*
> + * We must visit interior entries to get the hierarchy, but
> + * can skip not present mappings
> + */
> +for_each_pte(cs, _iterator, , true, false);
> +
> +/* Print last entry, if one present */
> +flush_print_pg_state(cs, );
> +}
> +
> static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
>hwaddr pte, hwaddr mask)
> {
> --
> 2.34.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
* Thomas Huth (th...@redhat.com) wrote: > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote: > > We are trying to unify all qemu-system-FOO to a single binary. > > In order to do that we need to remove QAPI target specific code. > > > > @dump-skeys is only available on qemu-system-s390x. This series > > rename it as @dump-s390-skey, making it available on other > > binaries. We take care of backward compatibility via deprecation. > > > > Philippe Mathieu-Daudé (4): > >hw/s390x: Introduce the @dump-s390-skeys QMP command > >hw/s390x: Introduce the 'dump_s390_skeys' HMP command > >hw/s390x: Deprecate the HMP 'dump_skeys' command > >hw/s390x: Deprecate the QMP @dump-skeys command > > Why do we have to rename the command? Just for the sake of it? I think > renaming HMP commands is maybe ok, but breaking the API in QMP is something > you should consider twice. > > And even if we decide to rename ... maybe we should discuss whether it makes > sense to come up with a generic command instead: As far as I know, ARM also > has something similar, called MTE. Maybe we also want to dump MTE keys one > day? So the new command should maybe be called "dump-memory-keys" instead? I think there are at least two different concepts; but I agree it would be nice to keep a single command for matching concepts across different architectures; I can't say I know the details of any, but: a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing where pages marked a special way are associated with keys. That sounds similar to what the skeys are??? b) Upper bit things - where you steal a few bits from the virtual address and then use that to associate some security; I think that's closer to what MTE is isn't it? I'm not sure the two fit in the same command. Dave > Or should it maybe rather be an option to the existing "dump-guest-memory" > command instead? > > Thomas > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH v2 3/4] tests/qtest/migration-test: Fix and enable test_ignore_shared
* Nicholas Piggin (npig...@gmail.com) wrote:
> This test is already starting to bitrot, so first remove it from ifdef
> and fix compile issues. ppc64 transfers about 2MB, so bump the size
> threshold too.
>
> It was said to be broken on aarch64 but it may have been due to the
> limited shm size under Gitlab CI. Now that it uses /tmp, enable it.
If it does fail, lets see if we can figure out how, i.e. whether it's the
shm size or something else.
Reviewed-by: Dr. David Alan Gilbert
> Cc: Yury Kotov
> Cc: Dr. David Alan Gilbert
> Signed-off-by: Nicholas Piggin
> ---
> tests/qtest/migration-test.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index de380757be..86eace354e 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1855,8 +1855,6 @@ static void
> test_precopy_unix_tls_x509_override_host(void)
> #endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> -#if 0
> -/* Currently upset on aarch64 TCG */
> static void test_ignore_shared(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1865,7 +1863,7 @@ static void test_ignore_shared(void)
> .use_memfile = true,
> };
>
> -if (test_migrate_start(, , uri, false, true, NULL, NULL)) {
> +if (test_migrate_start(, , uri, )) {
> return;
> }
>
> @@ -1890,11 +1888,11 @@ static void test_ignore_shared(void)
> wait_for_migration_complete(from);
>
> /* Check whether shared RAM has been really skipped */
> -g_assert_cmpint(read_ram_property_int(from, "transferred"), <, 1024 *
> 1024);
> +g_assert_cmpint(read_ram_property_int(from, "transferred"), <,
> + 4 * 1024 * 1024);
>
> test_migrate_end(from, to, true);
> }
> -#endif
>
> static void *
> test_migrate_xbzrle_start(QTestState *from,
> @@ -3535,7 +3533,8 @@ int main(int argc, char **argv)
> #endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> -/* migration_test_add("/migration/ignore_shared", test_ignore_shared); */
> +migration_test_add("/migration/ignore_shared", test_ignore_shared);
> +
> #ifndef _WIN32
> migration_test_add("/migration/precopy/fd/tcp",
> test_migrate_precopy_fd_socket);
> --
> 2.43.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] dm vdo: remove unused struct 'uds_attribute'
* Matthew Sakai (msa...@redhat.com) wrote:
> On 5/23/24 17:07, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'uds_attribute' is unused since
> > commit a9da0fb6d8c6 ("dm vdo: remove all sysfs interfaces").
> >
> > Remove it.
> >
> > Signed-off-by: Dr. David Alan Gilbert
>
> Yes, this was clearly an oversight on our part. Feel free to add:
> Reviewed-by: Matthew Sakai
Thanks for the quick review.
Is this something you'll send a pull for, or do I need to
ask someone else to include it?
Dave
> > ---
> > drivers/md/dm-vdo/dedupe.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/md/dm-vdo/dedupe.c b/drivers/md/dm-vdo/dedupe.c
> > index 117266e1b3ae..39ac68614419 100644
> > --- a/drivers/md/dm-vdo/dedupe.c
> > +++ b/drivers/md/dm-vdo/dedupe.c
> > @@ -148,11 +148,6 @@
> > #include "vdo.h"
> > #include "wait-queue.h"
> > -struct uds_attribute {
> > - struct attribute attr;
> > - const char *(*show_string)(struct hash_zones *hash_zones);
> > -};
> > -
> > #define DEDUPE_QUERY_TIMER_IDLE 0
> > #define DEDUPE_QUERY_TIMER_RUNNING 1
> > #define DEDUPE_QUERY_TIMER_FIRED 2
>
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] ubd: Remove unused mutex 'ubd_mutex'
* Richard Weinberger (rich...@nod.at) wrote: > - Ursprüngliche Mail - > > Von: "hch" > > An: "Dr. David Alan Gilbert" > > CC: "hch" , "richard" , "linux-um" > > , "linux-kernel" > > > > Gesendet: Montag, 20. Mai 2024 17:20:49 > > Betreff: Re: [PATCH] ubd: Remove unused mutex 'ubd_mutex' > > > On Mon, May 20, 2024 at 03:12:20PM +, Dr. David Alan Gilbert wrote: > >> * Christoph Hellwig (h...@lst.de) wrote: > >> > Looks good: > >> > > >> > Reviewed-by: Christoph Hellwig > >> > >> Thanks Christoph; any ideas who might take this? > > > > Probably Richard who is alredy on Cc. > > Yes, after the merge window. Thanks; no rush, just trying to keep an eye on where each patch is going. Dave > Thanks, > //richard > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH v2] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Mon, May 20, 2024 at 01:55:51PM +0100, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > has dropped all the users of the struct bridge_init from the
> > exynos_dp_core, while retaining unused structure definition.
> > Later on the driver was reworked and the definition migrated
> > to the analogix_dp driver. Remove unused struct bridge_init definition.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
>
> Reviewed-by: Dmitry Baryshkov
Thanks.
Dave
>
> --
> With best wishes
> Dmitry
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] ubd: Remove unused mutex 'ubd_mutex'
* Christoph Hellwig (h...@lst.de) wrote: > Looks good: > > Reviewed-by: Christoph Hellwig Thanks Christoph; any ideas who might take this? Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 3/7] linux-user: sparc: Remove unused struct 'target_mc_fq'
* Dr. David Alan Gilbert (d...@treblig.org) wrote:
> This struct is unused since Peter's
> Commit b8ae597f0e6d ("linux-user/sparc: Fix errors in target_ucontext
> structures")
>
> However, hmm, I'm a bit confused since that commit modifies the
> structure and then removes it, was that intentional?
Ping on this.
(I think the others in the set have been reviewed and one picked up).
Dave
> Signed-off-by: Dr. David Alan Gilbert
> ---
> linux-user/sparc/signal.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
> index f164b74032..8181b8b92c 100644
> --- a/linux-user/sparc/signal.c
> +++ b/linux-user/sparc/signal.c
> @@ -546,11 +546,6 @@ void setup_sigtramp(abi_ulong sigtramp_page)
> typedef abi_ulong target_mc_greg_t;
> typedef target_mc_greg_t target_mc_gregset_t[SPARC_MC_NGREG];
>
> -struct target_mc_fq {
> -abi_ulong mcfq_addr;
> -uint32_t mcfq_insn;
> -};
> -
> /*
> * Note the manual 16-alignment; the kernel gets this because it
> * includes a "long double qregs[16]" in the mcpu_fregs union,
> --
> 2.45.0
>
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Sun, May 19, 2024 at 10:43:44PM +0000, Dr. David Alan Gilbert wrote:
> > * Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> > > On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> > > > From: "Dr. David Alan Gilbert"
> > > >
> > > > 'bridge_init' is unused, I think following:
> > > > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > > > (which is where a git --follow finds it)
> > > > Remove it.
> > >
> > > Please rephrase the commit message following guidelines in
> > > Documentation/process. Use Fixes tags if suitable.
> >
> > I specifically don't want to use Fixes in these set because
> > there's no need for someone to backport this to older
> > kernels that use the original, and many backporters
> > use 'Fixes' as an automated means to find stuff they should
> > backport.
> >
> > Other than that, is there something specific you think I've
> > missed?
>
> It's not about missing things. It's about a way it is written.
> Consider something like:
>
> The commit aaa ("drm/bridge: foo bar") has dropped all the users of
> the struct bridge_init from the exynos_dp_core, while retainng unused
> structure definition. Later on the driver was reworked and the
> definition migrated to the analogix_dp driver. Remove unused struct
> bridge_init definition.
OK, v2 sent with text close to that.
>
> >
> > (I'm also purposely being less certain here, because --follow
> > is showing it in a ptn3460 and I don't quite follow
> > why that changes it here).
>
> The mentioned commit is a correct one. Historically exynos_dp_core had
> been creating the ptn3460 bridge manually. Later on this was fixed in
> the ptn3640 driver and the code was dropped from exynos_dp_core.
Ah OK; remember I don't know the actual structure of these devices
or the history.
Dave
> >
> > Dave
> >
> > >
> > > >
> > > > Build tested.
> > > >
> > > > Signed-off-by: Dr. David Alan Gilbert
> > > > ---
> > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > > > 1 file changed, 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > index df9370e0ff23..1e03f3525a92 100644
> > > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > @@ -36,11 +36,6 @@
> > > >
> > > > static const bool verify_fast_training;
> > > >
> > > > -struct bridge_init {
> > > > - struct i2c_client *client;
> > > > - struct device_node *node;
> > > > -};
> > > > -
> > > > static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> > > > {
> > > > int ret;
> > > > --
> > > > 2.45.1
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > --
> > -Open up your eyes, open up your mind, open up your code ---
> > / Dr. David Alan Gilbert| Running GNU/Linux | Happy \
> > \dave @ treblig.org | | In Hex /
> > \ _|_ http://www.treblig.org |___/
>
> --
> With best wishes
> Dmitry
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'bridge_init' is unused, I think following:
> > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > (which is where a git --follow finds it)
> > Remove it.
>
> Please rephrase the commit message following guidelines in
> Documentation/process. Use Fixes tags if suitable.
I specifically don't want to use Fixes in these set because
there's no need for someone to backport this to older
kernels that use the original, and many backporters
use 'Fixes' as an automated means to find stuff they should
backport.
Other than that, is there something specific you think I've
missed?
(I'm also purposely being less certain here, because --follow
is showing it in a ptn3460 and I don't quite follow
why that changes it here).
Dave
>
> >
> > Build tested.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index df9370e0ff23..1e03f3525a92 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -36,11 +36,6 @@
> >
> > static const bool verify_fast_training;
> >
> > -struct bridge_init {
> > - struct i2c_client *client;
> > - struct device_node *node;
> > -};
> > -
> > static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> > {
> > int ret;
> > --
> > 2.45.1
> >
>
> --
> With best wishes
> Dmitry
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: Build fail in mariadb on fedora 40
* Julien Nabet (serval2...@yahoo.fr) wrote:
> On 17/05/2024 15:47, Dr. David Alan Gilbert wrote:
> > Hi,
> >I'm getting:
> >
> > workdir/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.c:87:207:
> > error: initialization of ‘struct st_mysql_client_plugin *’ from
> > incompatible pointer type ‘struct st_mysql_client_plugin_AUTHENTICATION *’
> > [-Wincompatible-pointer-types]
> > 87 |(struct st_mysql_client_plugin *)_socket_client_plugin,
> > (struct st_mysql_client_plugin *)_sha2_password_client_plugin,
> > (struct st_mysql_client_plugin *)_native_password_client_plugin,
> > (struct st_mysql_client_plugin_AUTHENTICATION
> > *)_gssapi_client_client_plugin,
> >|
> >
> >^
> > /discs/fast/core/workdir/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.c:87:207:
> > note: (near initialization for ‘mysql_client_builtins[3]’)
> > make[1]: *** [/discs/fast/core/solenv/gbuild/LinkTarget.mk:366:
> > /discs/fast/core/workdir/GenCObject/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.o]
> > Error 1
> > make[1]: *** Waiting for unfinished jobs
> > make: *** [Makefile:294: build] Error 2
> >
> > Host: Fedora 40, x86-64
> > configured with:
> > ./configure --srcdir=/discs/fast/core --enable-option-checking=fatal
> > --enable-debug
> >
> > using --with-system-mariadb fixed it.
>
> Hello David,
Hi Julien,
Thanks for the reply.
> Sorry, I don't know why you encounter this :-(
It looks like others started hitting it; see:
https://gerrit.libreoffice.org/c/core/+/167806
(I believe as a windows user)
> If it can help, here's the content of my autogen.input:
>
> CC=clang
> CXX=clang++
Hmm I'm using gcc instead, but I don't see how the cast
would be valid:
UnpackedTarball/mariadb-connector-c/include/mysql/client_plugin.h has:
#include
struct st_mysql_client_plugin_AUTHENTICATION
{
MYSQL_CLIENT_PLUGIN_HEADER
int (*authenticate_user)(MYSQL_PLUGIN_VIO *vio, struct st_mysql *mysql);
};
although, hmm, that header also defines:
#define mysql_declare_client_plugin(X) \
struct st_mysql_client_plugin_ ## X\
_mysql_client_plugin_declaration_ = { \
MYSQL_CLIENT_ ## X ## _PLUGIN,\
MYSQL_CLIENT_ ## X ## _PLUGIN_INTERFACE_VERSION,
#define mysql_end_client_plugin }
which is curiously different from the system
/usr/include/mysql/server/mysql/client_plugin.h
#define mysql_declare_client_plugin(X) \
C_MODE_START MYSQL_PLUGIN_EXPORT_C \
struct st_mysql_client_plugin_ ## X\
_mysql_client_plugin_declaration_ = { \
MYSQL_CLIENT_ ## X ## _PLUGIN,\
MYSQL_CLIENT_ ## X ## _PLUGIN_INTERFACE_VERSION,
#define mysql_end_client_plugin }; C_MODE_END
so those C_MODE_START/END make me think someone is doing something
funky with the compiler anyway.
Dave
> --enable-ld=lld
> --enable-online-update
> --enable-dbgutil
> --enable-evolution2
> --enable-gtk4
> --enable-qt5
> --enable-kf5
> --enable-gtk3-kde5
> --enable-skia=debug
> --enable-ext-nlpsolver
> --enable-ext-numbertext
> --enable-ext-wiki-publisher
> --enable-dbus
> --enable-werror
> --enable-dependency-tracking
> --enable-python=fully-internal
> --without-system-mariadb
> --enable-bundle-mariadb
> --enable-symbols
> --enable-avahi
> --enable-eot
> --enable-odk
> --with-lang=en-US de es fr hu it ja nl pt pt-BR ru nb nn
> --with-myspell-dicts
>
> Julien
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
(oops the patch numbering in these 3 are wrong, they're all independent
patches)
Dave
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'bridge_init' is unused, I think following:
> commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> (which is where a git --follow finds it)
> Remove it.
>
> Build tested.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index df9370e0ff23..1e03f3525a92 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -36,11 +36,6 @@
>
> static const bool verify_fast_training;
>
> -struct bridge_init {
> - struct i2c_client *client;
> - struct device_node *node;
> -};
> -
> static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> int ret;
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] virt: acrn: Remove unusted list 'acrn_irqfd_clients'
* li...@treblig.org (li...@treblig.org) wrote: > From: "Dr. David Alan Gilbert" > > It doesn't look like this was ever used. > > Build tested only. > > Signed-off-by: Dr. David Alan Gilbert Ping > --- > drivers/virt/acrn/irqfd.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/virt/acrn/irqfd.c b/drivers/virt/acrn/irqfd.c > index d4ad211dce7a3..346cf0be4aac7 100644 > --- a/drivers/virt/acrn/irqfd.c > +++ b/drivers/virt/acrn/irqfd.c > @@ -16,8 +16,6 @@ > > #include "acrn_drv.h" > > -static LIST_HEAD(acrn_irqfd_clients); > - > /** > * struct hsm_irqfd - Properties of HSM irqfd > * @vm: Associated VM pointer > -- > 2.45.0 > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Build fail in mariadb on fedora 40
Hi, I'm getting: workdir/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.c:87:207: error: initialization of ‘struct st_mysql_client_plugin *’ from incompatible pointer type ‘struct st_mysql_client_plugin_AUTHENTICATION *’ [-Wincompatible-pointer-types] 87 |(struct st_mysql_client_plugin *)_socket_client_plugin, (struct st_mysql_client_plugin *)_sha2_password_client_plugin, (struct st_mysql_client_plugin *)_native_password_client_plugin, (struct st_mysql_client_plugin_AUTHENTICATION *)_gssapi_client_client_plugin, | ^ /discs/fast/core/workdir/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.c:87:207: note: (near initialization for ‘mysql_client_builtins[3]’) make[1]: *** [/discs/fast/core/solenv/gbuild/LinkTarget.mk:366: /discs/fast/core/workdir/GenCObject/UnpackedTarball/mariadb-connector-c/libmariadb/ma_client_plugin.o] Error 1 make[1]: *** Waiting for unfinished jobs make: *** [Makefile:294: build] Error 2 Host: Fedora 40, x86-64 configured with: ./configure --srcdir=/discs/fast/core --enable-option-checking=fatal --enable-debug using --with-system-mariadb fixed it. I originally filed this as: https://bugs.documentfoundation.org/show_bug.cgi?id=161141 However Ilmari pointed out that being a build bug it should be here. See the bug for my config.log. (cc'ing Julien since I see they did some stuff around there.) Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 6/7] target/ppc: Remove unused struct 'mmu_ctx_hash32'
* BALATON Zoltan (bala...@eik.bme.hu) wrote:
> On Sun, 5 May 2024, BALATON Zoltan wrote:
> > On Sun, 5 May 2024, Dr. David Alan Gilbert wrote:
> > > I think it's use was removed by
> > > Commit 5883d8b296 ("mmu-hash*: Don't use full ppc_hash{32,
> > > 64}_translate() path for get_phys_page_debug()")
> > >
> > > Signed-off-by: Dr. David Alan Gilbert
> >
> > Reviewed-by: BALATON Zoltan
>
> I've picked this patch up and will send with my series that changes the same
> place to avoid needing to rebase.
Thanks!
Dave
> Regards,
> BALATON Zoltan
>
> > > ---
> > > target/ppc/mmu-hash32.c | 6 --
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> > > index 3976416840..746321329c 100644
> > > --- a/target/ppc/mmu-hash32.c
> > > +++ b/target/ppc/mmu-hash32.c
> > > @@ -36,12 +36,6 @@
> > > # define LOG_BATS(...) do { } while (0)
> > > #endif
> > >
> > > -struct mmu_ctx_hash32 {
> > > -hwaddr raddr; /* Real address */
> > > -int prot; /* Protection bits */
> > > -int key; /* Access key */
> > > -};
> > > -
> > > static int ppc_hash32_pp_prot(int key, int pp, int nx)
> > > {
> > > int prot;
> > >
> >
> >
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: gitlab ssh down ?
* Enrico Weigelt, metux IT consult (i...@metux.net) wrote: > Hi folks, > > I'm just having trouble pushing to my own forks via ssh. It's also > really slow, so I suspect some overload problem. Remaining for some > hours now. > > Anybody else noticing similar problems ? Yeh, I'm seeing that (on the mesa part). Dave > > --mtx > -- > --- > Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert > werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren > GPG/PGP-Schlüssel zu. > --- > Enrico Weigelt, metux IT consult > Free software and Linux embedded engineering > i...@metux.net -- +49-151-27565287 -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH] ftrace: Remove unused global 'ftrace_direct_func_count'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> Commit 8788ca164eb4b ("ftrace: Remove the legacy _ftrace_direct API")
> stopped setting the 'ftrace_direct_func_count' variable, but left
> it around. Clean it up.
>
> Signed-off-by: Dr. David Alan Gilbert
FYI this is on top of my earlier 'ftrace: Remove unused list
'ftrace_direct_funcs'
Dave
> ---
> include/linux/ftrace.h | 2 --
> kernel/trace/fgraph.c | 11 ---
> kernel/trace/ftrace.c | 1 -
> 3 files changed, 14 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index b01cca36147ff..e3a83ebd1b333 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -413,7 +413,6 @@ struct ftrace_func_entry {
> };
>
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> -extern int ftrace_direct_func_count;
> unsigned long ftrace_find_rec_direct(unsigned long ip);
> int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
> int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
> @@ -425,7 +424,6 @@ void ftrace_stub_direct_tramp(void);
>
> #else
> struct ftrace_ops;
> -# define ftrace_direct_func_count 0
> static inline unsigned long ftrace_find_rec_direct(unsigned long ip)
> {
> return 0;
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index c83c005e654e3..a130b2d898f7c 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -125,17 +125,6 @@ int function_graph_enter(unsigned long ret, unsigned
> long func,
> {
> struct ftrace_graph_ent trace;
>
> -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> - /*
> - * Skip graph tracing if the return location is served by direct
> trampoline,
> - * since call sequence and return addresses are unpredictable anyway.
> - * Ex: BPF trampoline may call original function and may skip frame
> - * depending on type of BPF programs attached.
> - */
> - if (ftrace_direct_func_count &&
> - ftrace_find_rec_direct(ret - MCOUNT_INSN_SIZE))
> - return -EBUSY;
> -#endif
> trace.func = func;
> trace.depth = ++current->curr_ret_depth;
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index b18b4ece3d7c9..adf34167c3418 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2538,7 +2538,6 @@ ftrace_find_unique_ops(struct dyn_ftrace *rec)
> /* Protected by rcu_tasks for reading, and direct_mutex for writing */
> static struct ftrace_hash __rcu *direct_functions = EMPTY_HASH;
> static DEFINE_MUTEX(direct_mutex);
> -int ftrace_direct_func_count;
>
> /*
> * Search the direct_functions hash to see if the given instruction pointer
> --
> 2.45.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: ftrace_direct_func_count ?
* Steven Rostedt (rost...@goodmis.org) wrote:
> On Sat, 4 May 2024 13:35:26 +
> "Dr. David Alan Gilbert" wrote:
>
> > Hi,
> > I've just posted a patch 'ftrace: Remove unused list
> > 'ftrace_direct_funcs''
> > that clears out some old code, but while at it I noticed the global
> > 'ftrace_direct_func_count'.
> >
> > As far as I can tell, it's never assigned (or initialised) but it is tested:
> >
> > kernel/trace/fgraph.c:
> > #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> > /*
> >* Skip graph tracing if the return location is served by direct
> > trampoline,
> >* since call sequence and return addresses are unpredictable anyway.
> >* Ex: BPF trampoline may call original function and may skip frame
> >* depending on type of BPF programs attached.
> >*/
> > if (ftrace_direct_func_count &&
> > ftrace_find_rec_direct(ret - MCOUNT_INSN_SIZE))
> > return -EBUSY;
> > #endif
> >
> > So I wasn't sure whether it was just safe to nuke that section
> > or whether it really needed fixing?
>
> Yes, after commit 8788ca164eb4bad ("ftrace: Remove the legacy
> _ftrace_direct API") that variable is no longer used.
OK, thanks, I'll send a follow up patch to my other patch to nuke
this as well.
Dave
> -- Steve
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 2/7] linux-user: i386/signal: Remove unused fp structs
* Richard Henderson (richard.hender...@linaro.org) wrote:
> On 5/5/24 10:14, Dr. David Alan Gilbert wrote:
> > The structs 'target_fpxreg' and 'target_xmmreg' are unused since
> > Paolo's:
> >
> > Commit 2796f290b546 ("linux-user: i386/signal: support FXSAVE fpstate on
> > 32-bit emulation")
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > linux-user/i386/signal.c | 10 --
> > 1 file changed, 10 deletions(-)
>
> I have
>
> https://lore.kernel.org/qemu-devel/20240409050302.1523277-1-richard.hender...@linaro.org/
>
> which cleans this up. I need to refresh and resubmit.
Ah yes, that's fine.
Dave
>
>
> r~
>
> >
> > diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
> > index 990048f42a..9bf602b388 100644
> > --- a/linux-user/i386/signal.c
> > +++ b/linux-user/i386/signal.c
> > @@ -34,16 +34,6 @@ struct target_fpreg {
> > uint16_t exponent;
> > };
> > -struct target_fpxreg {
> > -uint16_t significand[4];
> > -uint16_t exponent;
> > -uint16_t padding[3];
> > -};
> > -
> > -struct target_xmmreg {
> > -uint32_t element[4];
> > -};
> > -
> > struct target_fpx_sw_bytes {
> > uint32_t magic1;
> > uint32_t extended_size;
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
[PATCH 3/7] linux-user: sparc: Remove unused struct 'target_mc_fq'
This struct is unused since Peter's
Commit b8ae597f0e6d ("linux-user/sparc: Fix errors in target_ucontext
structures")
However, hmm, I'm a bit confused since that commit modifies the
structure and then removes it, was that intentional?
Signed-off-by: Dr. David Alan Gilbert
---
linux-user/sparc/signal.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index f164b74032..8181b8b92c 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -546,11 +546,6 @@ void setup_sigtramp(abi_ulong sigtramp_page)
typedef abi_ulong target_mc_greg_t;
typedef target_mc_greg_t target_mc_gregset_t[SPARC_MC_NGREG];
-struct target_mc_fq {
-abi_ulong mcfq_addr;
-uint32_t mcfq_insn;
-};
-
/*
* Note the manual 16-alignment; the kernel gets this because it
* includes a "long double qregs[16]" in the mcpu_fregs union,
--
2.45.0
[PATCH 2/7] linux-user: i386/signal: Remove unused fp structs
The structs 'target_fpxreg' and 'target_xmmreg' are unused since
Paolo's:
Commit 2796f290b546 ("linux-user: i386/signal: support FXSAVE fpstate on
32-bit emulation")
Signed-off-by: Dr. David Alan Gilbert
---
linux-user/i386/signal.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index 990048f42a..9bf602b388 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -34,16 +34,6 @@ struct target_fpreg {
uint16_t exponent;
};
-struct target_fpxreg {
-uint16_t significand[4];
-uint16_t exponent;
-uint16_t padding[3];
-};
-
-struct target_xmmreg {
-uint32_t element[4];
-};
-
struct target_fpx_sw_bytes {
uint32_t magic1;
uint32_t extended_size;
--
2.45.0
[PATCH 1/7] linux-user: cris: Remove unused struct 'rt_signal_frame'
Since 'setup_rt_frame' has never been implemented, this struct
is unused.
Signed-off-by: Dr. David Alan Gilbert
---
linux-user/cris/signal.c | 8
1 file changed, 8 deletions(-)
diff --git a/linux-user/cris/signal.c b/linux-user/cris/signal.c
index 4f532b2903..10948bcf30 100644
--- a/linux-user/cris/signal.c
+++ b/linux-user/cris/signal.c
@@ -35,14 +35,6 @@ struct target_signal_frame {
uint16_t retcode[4]; /* Trampoline code. */
};
-struct rt_signal_frame {
-siginfo_t *pinfo;
-void *puc;
-siginfo_t info;
-ucontext_t uc;
-uint16_t retcode[4]; /* Trampoline code. */
-};
-
static void setup_sigcontext(struct target_sigcontext *sc, CPUCRISState *env)
{
__put_user(env->regs[0], >regs.r0);
--
2.45.0
[PATCH 0/7] Remove some unused structures
A bunch of structs that are currently unused, found with a simple script and a bit of eyeballing. The only one I'm that suspicious of is the SPARC one, where the patch which removed the use is a bit confusing to me. Tested with a --target-list=ppc-softmmu,m68k-softmmu,x86_64-softmmu,arm-softmmu,aarch64-softmmu,cris-linux-user,i386-linux-user,sparc-linux-user and 'make check' on x86 linux. Dave Dr. David Alan Gilbert (7): linux-user: cris: Remove unused struct 'rt_signal_frame' linux-user: i386/signal: Remove unused fp structs linux-user: sparc: Remove unused struct 'target_mc_fq' hw/usb/dev-network: Remove unused struct 'rndis_config_parameter' hw/arm/bcm2836: Remove unusued struct 'BCM283XClass' target/ppc: Remove unused struct 'mmu_ctx_hash32' net/can: Remove unused struct 'CanBusState' hw/arm/bcm2836.c | 12 hw/usb/dev-network.c | 8 linux-user/cris/signal.c | 8 linux-user/i386/signal.c | 10 -- linux-user/sparc/signal.c | 5 - net/can/can_host.c| 6 -- target/ppc/mmu-hash32.c | 6 -- 7 files changed, 55 deletions(-) -- 2.45.0
[PATCH 7/7] net/can: Remove unused struct 'CanBusState'
As far as I can tell this struct has never been used in this
file (it is used in can_core.c).
Signed-off-by: Dr. David Alan Gilbert
---
net/can/can_host.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/net/can/can_host.c b/net/can/can_host.c
index a3c84028c6..b2fe553f91 100644
--- a/net/can/can_host.c
+++ b/net/can/can_host.c
@@ -34,12 +34,6 @@
#include "net/can_emu.h"
#include "net/can_host.h"
-struct CanBusState {
-Object object;
-
-QTAILQ_HEAD(, CanBusClientState) clients;
-};
-
static void can_host_disconnect(CanHostState *ch)
{
CanHostClass *chc = CAN_HOST_GET_CLASS(ch);
--
2.45.0
[PATCH 5/7] hw/arm/bcm2836: Remove unusued struct 'BCM283XClass'
This struct has been unused since
Commit f932093ae165 ("hw/arm/bcm2836: Split out common part of BCM283X
classes")
Signed-off-by: Dr. David Alan Gilbert
---
hw/arm/bcm2836.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index db191661f2..40a379bc36 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -18,18 +18,6 @@
#include "target/arm/cpu-qom.h"
#include "target/arm/gtimer.h"
-struct BCM283XClass {
-/*< private >*/
-DeviceClass parent_class;
-/*< public >*/
-const char *name;
-const char *cpu_type;
-unsigned core_count;
-hwaddr peri_base; /* Peripheral base address seen by the CPU */
-hwaddr ctrl_base; /* Interrupt controller and mailboxes etc. */
-int clusterid;
-};
-
static Property bcm2836_enabled_cores_property =
DEFINE_PROP_UINT32("enabled-cpus", BCM283XBaseState, enabled_cpus, 0);
--
2.45.0
[PATCH 6/7] target/ppc: Remove unused struct 'mmu_ctx_hash32'
I think it's use was removed by
Commit 5883d8b296 ("mmu-hash*: Don't use full ppc_hash{32,
64}_translate() path for get_phys_page_debug()")
Signed-off-by: Dr. David Alan Gilbert
---
target/ppc/mmu-hash32.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 3976416840..746321329c 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -36,12 +36,6 @@
# define LOG_BATS(...) do { } while (0)
#endif
-struct mmu_ctx_hash32 {
-hwaddr raddr; /* Real address */
-int prot; /* Protection bits */
-int key; /* Access key*/
-};
-
static int ppc_hash32_pp_prot(int key, int pp, int nx)
{
int prot;
--
2.45.0
[PATCH 4/7] hw/usb/dev-network: Remove unused struct 'rndis_config_parameter'
As far as I can tell it was never used.
Signed-off-by: Dr. David Alan Gilbert
---
hw/usb/dev-network.c | 8
1 file changed, 8 deletions(-)
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 2c33e36cad..d00d68b21d 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -475,14 +475,6 @@ struct rndis_packet_msg_type {
le32 Reserved;
};
-struct rndis_config_parameter {
-le32 ParameterNameOffset;
-le32 ParameterNameLength;
-le32 ParameterType;
-le32 ParameterValueOffset;
-le32 ParameterValueLength;
-};
-
/* implementation specific */
enum rndis_state
{
--
2.45.0
ftrace_direct_func_count ?
Hi, I've just posted a patch 'ftrace: Remove unused list 'ftrace_direct_funcs'' that clears out some old code, but while at it I noticed the global 'ftrace_direct_func_count'. As far as I can tell, it's never assigned (or initialised) but it is tested: kernel/trace/fgraph.c: #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS /* * Skip graph tracing if the return location is served by direct trampoline, * since call sequence and return addresses are unpredictable anyway. * Ex: BPF trampoline may call original function and may skip frame * depending on type of BPF programs attached. */ if (ftrace_direct_func_count && ftrace_find_rec_direct(ret - MCOUNT_INSN_SIZE)) return -EBUSY; #endif So I wasn't sure whether it was just safe to nuke that section or whether it really needed fixing? Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH v3 3/6] migration: Remove 'blk/-b' option from migrate commands
* Markus Armbruster (arm...@redhat.com) wrote:
> Fabiano Rosas writes:
>
> > The block migration is considered obsolete and has been deprecated in
> > 8.2. Remove the migrate command option that enables it. This only
> > affects the QMP and HMP commands, the feature can still be accessed by
> > setting the migration 'block' capability. The whole feature will be
> > removed in a future patch.
> >
> > Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
> > option is deprecated.").
> >
> > Reviewed-by: Markus Armbruster
> > Signed-off-by: Fabiano Rosas
>
> [...]
>
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 7978302949..ebca2cdced 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -909,21 +909,17 @@ ERST
> >
> > {
> > .name = "migrate",
> > -.args_type = "detach:-d,blk:-b,resume:-r,uri:s",
> > -.params = "[-d] [-b] [-r] uri",
> > +.args_type = "detach:-d,resume:-r,uri:s",
> > +.params = "[-d] [-r] uri",
> > .help = "migrate to URI (using -d to not wait for
> > completion)"
> > - "\n\t\t\t -b for migration without shared storage with"
> > - " full copy of disk\n\t\t\t -r to resume a paused
> > migration",
> > + "\n\t\t\t -r to resume a paused migration",
> > .cmd= hmp_migrate,
> > },
> >
> >
> > SRST
> > -``migrate [-d] [-b]`` *uri*
> > +``migrate [-d]`` *uri*
> >Migrate to *uri* (using -d to not wait for completion).
> > -
> > - ``-b``
> > -for migration with full copy of disk
> > ERST
>
> Not this patch's fault, but here goes anyway: -r is undocumented here.
Probably one for Peter I guess.
Dave
> >
> > {
>
> [...]
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v3 3/6] migration: Remove 'blk/-b' option from migrate commands
* Markus Armbruster (arm...@redhat.com) wrote:
> Fabiano Rosas writes:
>
> > The block migration is considered obsolete and has been deprecated in
> > 8.2. Remove the migrate command option that enables it. This only
> > affects the QMP and HMP commands, the feature can still be accessed by
> > setting the migration 'block' capability. The whole feature will be
> > removed in a future patch.
> >
> > Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
> > option is deprecated.").
> >
> > Reviewed-by: Markus Armbruster
> > Signed-off-by: Fabiano Rosas
>
> [...]
>
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 7978302949..ebca2cdced 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -909,21 +909,17 @@ ERST
> >
> > {
> > .name = "migrate",
> > -.args_type = "detach:-d,blk:-b,resume:-r,uri:s",
> > -.params = "[-d] [-b] [-r] uri",
> > +.args_type = "detach:-d,resume:-r,uri:s",
> > +.params = "[-d] [-r] uri",
> > .help = "migrate to URI (using -d to not wait for
> > completion)"
> > - "\n\t\t\t -b for migration without shared storage with"
> > - " full copy of disk\n\t\t\t -r to resume a paused
> > migration",
> > + "\n\t\t\t -r to resume a paused migration",
> > .cmd= hmp_migrate,
> > },
> >
> >
> > SRST
> > -``migrate [-d] [-b]`` *uri*
> > +``migrate [-d]`` *uri*
> >Migrate to *uri* (using -d to not wait for completion).
> > -
> > - ``-b``
> > -for migration with full copy of disk
> > ERST
>
> Not this patch's fault, but here goes anyway: -r is undocumented here.
Probably one for Peter I guess.
Dave
> >
> > {
>
> [...]
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: Add 'info pg' command to monitor
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Tue, 16 Apr 2024 at 19:11, Don Porter wrote: > > > > On 4/16/24 13:03, Peter Maydell wrote: > > > On Tue, 16 Apr 2024 at 17:53, Don Porter wrote: > > >> There is still a lot I am learning about the code base, but it seems > > >> that qemu_get_guest_memory_mapping() does most of what one would need. > > >> It currently only returns the "leaves" of the page table tree in a list. > > >> > > >> What if I extend this function with an optional argument to either > > >> 1) return the interior nodes of the page table in additional lists (and > > >> then parse+print in the monitor code), or > > >> 2) inline the monitor printing in the arch-specific hook, and pass a > > >> flag to get_guest_memory_mapping() that turns on/off the statements that > > >> pretty print the page tables? > > >> > > >> It looks like most CPUs implement this function as part of checkpointing. > > > As far as I can see only x86 implements the get_memory_mapping > > > function, so once again somebody has added some bit of > > > functionality that does a walk of the page tables that is > > > x86 only and that shares no code with any of the other > > > page table walking code :-( > > > > My mistake - get_memory_mappings() is only implemented in x86. > > > > In doing some searching of the code, many architectures implement > > mmu_translate() and > > get_physical_address() functions, but they are not standardized. I also > > see your larger point > > about replicating page walking code in x86. > > > > I imagine you have something in mind that abstracts things like the > > height of the radix tree, > > entries per node, checking permissions, printing the contents, etc. > > > > Perhaps I should start by trying to merge the x86 page walking code into > > one set of common > > helper functions, get more feedback (perhaps on a new patch thread?), > > and then consider > > how to abstract across architectures after getting feedback on this? > > I think the cross-architecture abstraction is probably the > trickiest part. I would actually be happy for us to drop > 'info tlb' and 'info mem' entirely if we have a cross-arch > command that gives basically the same information -- we don't > IMHO need more than one command for this, and we only have > multiple commands for basically legacy reasons. And for the > human monitor (HMP) we don't need to keep things around > for backwards compatibility. I'm not sure what happens for the (MIPS/SPARC ?) where it's not a traditional table hierarchy. The other thing you might want (and I'm not sure how it interacts with any of this) is to specify the root of the MMU tree (i.e. CR3 value for those in Intel thinking) to dump different processes etc. Dave > thanks > -- PMM -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: Add 'info pg' command to monitor
* Don Porter (por...@cs.unc.edu) wrote: > On 4/16/24 13:03, Peter Maydell wrote: > > On Tue, 16 Apr 2024 at 17:53, Don Porter wrote: > > > There is still a lot I am learning about the code base, but it seems > > > that qemu_get_guest_memory_mapping() does most of what one would need. > > > It currently only returns the "leaves" of the page table tree in a list. > > > > > > What if I extend this function with an optional argument to either > > > 1) return the interior nodes of the page table in additional lists (and > > > then parse+print in the monitor code), or > > > 2) inline the monitor printing in the arch-specific hook, and pass a > > > flag to get_guest_memory_mapping() that turns on/off the statements that > > > pretty print the page tables? > > > > > > It looks like most CPUs implement this function as part of checkpointing. > > As far as I can see only x86 implements the get_memory_mapping > > function, so once again somebody has added some bit of > > functionality that does a walk of the page tables that is > > x86 only and that shares no code with any of the other > > page table walking code :-( > > My mistake - get_memory_mappings() is only implemented in x86. > > In doing some searching of the code, many architectures implement > mmu_translate() and > get_physical_address() functions, but they are not standardized. I also see > your larger point > about replicating page walking code in x86. > > I imagine you have something in mind that abstracts things like the height > of the radix tree, > entries per node, checking permissions, printing the contents, etc. > > Perhaps I should start by trying to merge the x86 page walking code into one > set of common > helper functions, get more feedback (perhaps on a new patch thread?), and > then consider > how to abstract across architectures after getting feedback on this? > > In looking at x86 code, I see the following places where there is page table > walking code to > potentially merge: > > * target/i386/monitor.c - existing info commands > > * target/i386/helper.c - get_phys_page_attrs_debug > > * target/i386/arch_memory_mapping.c - implements get_memory_mapping > > * tcg/sysemu/excp_helper.c: implements mmu_translate() and > get_physical_address() One thing to keep in mind (although I don't know the x86 code) is that you want the monitor command not to change any state, nor to fail if there's a particularly screwy page table; so no flagging exceptions or flagging accessed bits or changing the state of the tcg. Dave > Thanks, > > Don > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH] module: ban '.', '..' as module names, ban '/' in module names
* Alexey Dobriyan (adobri...@gmail.com) wrote:
> On Sun, Apr 14, 2024 at 01:58:55PM -0700, Luis Chamberlain wrote:
> > On Sun, Apr 14, 2024 at 10:05:05PM +0300, Alexey Dobriyan wrote:
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -3616,4 +3616,12 @@ extern int vfs_fadvise(struct file *file, loff_t
> > > offset, loff_t len,
> > > extern int generic_fadvise(struct file *file, loff_t offset, loff_t len,
> > > int advice);
> > >
> > > +/*
> > > + * Use this if data from userspace end up as directory/filename on
> > > + * some virtual filesystem.
> > > + */
> > > +static inline bool string_is_vfs_ready(const char *s)
> > > +{
> > > + return strcmp(s, ".") != 0 && strcmp(s, "..") != 0 && !strchr(s, '/');
> > > +}
> > > #endif /* _LINUX_FS_H */
> > > --- a/kernel/module/main.c
> > > +++ b/kernel/module/main.c
> > > @@ -2893,6 +2893,11 @@ static int load_module(struct load_info *info,
> > > const char __user *uargs,
> > >
> > > audit_log_kern_module(mod->name);
> > >
> > > + if (!string_is_vfs_ready(mod->name)) {
> > > + err = -EINVAL;
> > > + goto free_module;
> > > + }
> > > +
> >
> > Sensible change however to put string_is_vfs_ready() in include/linux/fs.h
> > is a stretch if there really are no other users.
>
> This is forward thinking patch :-)
>
> Other subsystems may create files/directories in proc/sysfs, and should
> check for bad names as well:
>
> /proc/2821/net/dev_snmp6/eth0
>
> This looks exactly like something coming from userspace and making it
> into /proc, so the filter function doesn't belong to kernel/module/internal.h
You mean like:
[24180.292204] tuxthe: renamed from tuxthe
root@dalek:/home/dg# ls /sys/class/net/
enp5s0 lo tuxthe tuxthe tuxthe virbr0 virbr1
?
Dave
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: Isolating qa data?
* Miklos Vajna (vmik...@collabora.com) wrote: > Hi Dave, > > On Wed, Apr 03, 2024 at 03:26:42PM +0000, "Dr. David Alan Gilbert" > wrote: > > What I'm thinking is that a cautious packager could set it up so > > that during the build qa/data wasn't readable, then having built and > > copied their installation out, then they could perform a make check > > having reenabled access to qa/data/ > > I wonder if this would be doable also at the current setup. Something > like search for all "data" directories under */qa/, delete them, do the > build, 'git checkout -f' to do undo the delete and then 'make check'? Yeh, I guess possible; I'm not sure what's easiest for packagers. (I was thinking along the lines of a permissions or blank directory mounted over it). Actually, a quick hunt for where we have docs shows they're a bit more spread around; e.g. ./sw/qa/core/exportdata/rtf/pass ./sc/qa/extras/testdocuments ./odk/examples/basic/sheet ./pyuno/qa/pytests/testdocuments so a simplefind . -path .*qa/*/data isn't sufficient. Dave > > Regards, > > Miklos -- -----Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Isolating qa data?
Hi, Given the xz hack, would it make sense to isolate the qa data files under a single directory, e.g. qa/data/vcl/cppunit/pdfexport rather than vcl/qa/cppunit/pdfexport ? What I'm thinking is that a cautious packager could set it up so that during the build qa/data wasn't readable, then having built and copied their installation out, then they could perform a make check having reenabled access to qa/data/ To be clear I'm not worrying particularly about LO, but fundamentally a lot of the test files start off as random bug reports and it's rare to look inside the test files for anything odd. Just an idea, Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
* BALATON Zoltan (bala...@eik.bme.hu) wrote:
> On Sun, 24 Mar 2024, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> > > Replace qemu_printf() by monitor_printf() / monitor_puts() in monitor.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé
> > > ---
> > > target/m68k/cpu.h | 2 +-
> > > target/m68k/helper.c | 126 +-
> > > target/m68k/monitor.c | 4 +-
> > > 3 files changed, 67 insertions(+), 65 deletions(-)
> > >
> > > diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> > > index 346427e144..4e4307956d 100644
> > > --- a/target/m68k/cpu.h
> > > +++ b/target/m68k/cpu.h
> > > @@ -620,6 +620,6 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState
> > > *env, vaddr *pc,
> > > }
> > > }
> > >
> > > -void dump_mmu(CPUM68KState *env);
> > > +void dump_mmu(Monitor *mon, CPUM68KState *env);
> > >
> > > #endif
> > > diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> > > index 1a475f082a..310e26dfa1 100644
> > > --- a/target/m68k/helper.c
> > > +++ b/target/m68k/helper.c
> > > @@ -25,7 +25,7 @@
> > > #include "exec/helper-proto.h"
> > > #include "gdbstub/helpers.h"
> > > #include "fpu/softfloat.h"
> > > -#include "qemu/qemu-print.h"
> > > +#include "monitor/monitor.h"
> > >
> > > #define SIGNBIT (1u << 31)
> > >
> > > @@ -455,28 +455,30 @@ void m68k_switch_sp(CPUM68KState *env)
> > > #if !defined(CONFIG_USER_ONLY)
> > > /* MMU: 68040 only */
> > >
> > > -static void print_address_zone(uint32_t logical, uint32_t physical,
> > > +static void print_address_zone(Monitor *mon,
> > > + uint32_t logical, uint32_t physical,
> > > uint32_t size, int attr)
> > > {
> > > -qemu_printf("%08x - %08x -> %08x - %08x %c ",
> > > -logical, logical + size - 1,
> > > -physical, physical + size - 1,
> > > -attr & 4 ? 'W' : '-');
> > > +monitor_printf(mon, "%08x - %08x -> %08x - %08x %c ",
> > > + logical, logical + size - 1,
> > > + physical, physical + size - 1,
> > > + attr & 4 ? 'W' : '-');
> > > size >>= 10;
> > > if (size < 1024) {
> > > -qemu_printf("(%d KiB)\n", size);
> > > +monitor_printf(mon, "(%d KiB)\n", size);
> > > } else {
> > > size >>= 10;
> > > if (size < 1024) {
> > > -qemu_printf("(%d MiB)\n", size);
> > > +monitor_printf(mon, "(%d MiB)\n", size);
> > > } else {
> > > size >>= 10;
> > > -qemu_printf("(%d GiB)\n", size);
> > > +monitor_printf(mon, "(%d GiB)\n", size);
> > > }
> > > }
> > > }
> > >
> > > -static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
> > > +static void dump_address_map(Monitor *mon, CPUM68KState *env,
> > > + uint32_t root_pointer)
> > > {
> > > int i, j, k;
> > > int tic_size, tic_shift;
> > > @@ -545,7 +547,7 @@ static void dump_address_map(CPUM68KState *env,
> > > uint32_t root_pointer)
> > > if (first_logical != 0x) {
> > > size = last_logical + (1 << tic_shift) -
> > > first_logical;
> > > -print_address_zone(first_logical,
> > > +print_address_zone(mon, first_logical,
> > > first_physical, size,
> > > last_attr);
> > > }
> > > first_logical = logical;
> > > @@ -556,125 +558,125 @@ static void dump_address_map(CPUM68KState *env,
> > > uint32_t root_pointer)
> > > }
> > > if (first_logical != logical || (attr & 4) != (last_attr & 4)) {
> > > size = logical + (1 << tic_shift) - first_logical;
> > > -print_address_zone(first_logi
Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu'
* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Wed, 20 Mar 2024 at 17:06, Philippe Mathieu-Daudé
> wrote:
> >
> > +Alex/Daniel
> >
> > On 20/3/24 17:53, Peter Maydell wrote:
> > > On Wed, 20 Mar 2024 at 16:40, Philippe Mathieu-Daudé
> > > wrote:
> > >>
> > >> 'info tlb' and 'info mem' commands don't scale in heterogeneous
> > >> emulation. They will be reworked after the next release, hidden
> > >> behind the 'info mmu' command. It is not too late to deprecate
> > >> commands, so add the 'info mmu' command as wrapper to the other
> > >> ones, but already deprecate them.
> > >>
> > >> Philippe Mathieu-Daudé (2):
> > >>target/monitor: Introduce 'info mmu' command
> > >>target/monitor: Deprecate 'info tlb' and 'info mem' commands
> > >
> > > This seems to replace "info tlb" and "info mem" with "info mmu -t"
> > > and "info mmu -m", but it doesn't really say anything about:
> > > * what the difference is between these two things
> >
> > I really don't know; I'm only trying to keep the monitor interface
> > identical.
>
> You don't, though: you change it from "info tlb" to "info mmu -t" etc.
>
> > > * which targets implement which and why
> >
> > This one is easy to answer:
> >
> > #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC)
> > || \
> > defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
> > {
> > .name = "tlb",
> >
> > #if defined(TARGET_I386) || defined(TARGET_RISCV)
> > {
> > .name = "mem",
> >
> > > * what the plan is for the future
> >
> > My problem is with linking a single QEMU binary, as these two symbols
> > (hmp_info_mem and hmp_info_tlb) clash.
>
> Yes, but they both (implicitly) operate on the current HMP CPU,
> so the problem with linking into a single binary is that they're
> not indirected through a method on the CPU object, not the syntax
> used in the monitor to invoke them, presumably.
>
> > I'm indeed only postponing the problem, without looking at what
> > this code does. I did it adding hmp_info_mmu_tlb/mem hooks in
> > TCGCPUOps ("hw/core/tcg-cpu-ops.h"), so the command can be
> > dispatched per target vcpu as target-agnostic code in
> > monitor/hmp-cmds.c:
> >
> > +#include "hw/core/tcg-cpu-ops.h"
> > +
> > +static void hmp_info_mmu_tlb(Monitor *mon, CPUState *cpu)
> > +{
> > +const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
> > +
> > +if (tcg_ops->hmp_info_mmu_tlb) {
> > +tcg_ops->hmp_info_mmu_tlb(mon, cpu_env(cpu));
> > +} else {
> > +monitor_puts(mon, "No per-CPU information available on this
> > target\n");
> > +}
> > +}
>
> These aren't TCG specific though, so why TCGCPUOps ?
>
> > > I am definitely not a fan of either of these commands, because
> > > (as we currently implement them) they effectively require each
> > > target architecture to implement a second copy of the page table
> > > walking code. But before we can deprecate them we need to be
> > > pretty sure that "info mmu" is what we want to replace them with.
> >
> > An alternative is to just deprecate them, without adding "info mmu" :)
> >
> > It is OK to un-deprecate stuff if we realize its usefulness.
>
> The commands are there because some users find them useful.
> I just dislike them because I think they're a bit niche and
> annoying to implement and not consistent across target
> architectures and not very well documented...
>
> By the way, we have no obligation to follow the deprecate-and-drop
> process for HMP commands; unlike QMP, we give ourselves the
> license to vary it when we feel like it, because the users are
> humans, not programs or scripts.
Right, so no rush to get the deprecation in; change it when you agree
what you'd like a replacement to look like.
Dave
> -- PMM
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
(Monitor *mon, uint32_t ttr)
> {
> if ((ttr & M68K_TTR_ENABLED) == 0) {
> -qemu_printf("disabled\n");
> +monitor_puts(mon, "disabled\n");
> return;
> }
> -qemu_printf("Base: 0x%08x Mask: 0x%08x Control: ",
> -ttr & M68K_TTR_ADDR_BASE,
> -(ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> +monitor_printf(mon, "Base: 0x%08x Mask: 0x%08x Control: ",
> + ttr & M68K_TTR_ADDR_BASE,
> + (ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> switch (ttr & M68K_TTR_SFIELD) {
> case M68K_TTR_SFIELD_USER:
> -qemu_printf("U");
> +monitor_puts(mon, "U");
> break;
> case M68K_TTR_SFIELD_SUPER:
> -qemu_printf("S");
> +monitor_puts(mon, "S");
> break;
> default:
> -qemu_printf("*");
> +monitor_puts(mon, "*");
> break;
> }
> DUMP_CACHEFLAGS(ttr);
> if (ttr & M68K_DESC_WRITEPROT) {
> -qemu_printf("R");
> +monitor_puts(mon, "R");
> } else {
> -qemu_printf("W");
> +monitor_puts(mon, "W");
> }
> -qemu_printf(" U: %d\n", (ttr & M68K_DESC_USERATTR) >>
> +monitor_printf(mon, " U: %d\n", (ttr & M68K_DESC_USERATTR) >>
> M68K_DESC_USERATTR_SHIFT);
> }
>
> -void dump_mmu(CPUM68KState *env)
> +void dump_mmu(Monitor *mon, CPUM68KState *env)
> {
> if ((env->mmu.tcr & M68K_TCR_ENABLED) == 0) {
> -qemu_printf("Translation disabled\n");
> +monitor_puts(mon, "Translation disabled\n");
> return;
> }
> -qemu_printf("Page Size: ");
> +monitor_puts(mon, "Page Size: ");
> if (env->mmu.tcr & M68K_TCR_PAGE_8K) {
> -qemu_printf("8kB\n");
> +monitor_puts(mon, "8kB\n");
> } else {
> -qemu_printf("4kB\n");
> +monitor_puts(mon, "4kB\n");
> }
>
> -qemu_printf("MMUSR: ");
> +monitor_puts(mon, "MMUSR: ");
> if (env->mmu.mmusr & M68K_MMU_B_040) {
> -qemu_printf("BUS ERROR\n");
> +monitor_puts(mon, "BUS ERROR\n");
> } else {
> -qemu_printf("Phy=%08x Flags: ", env->mmu.mmusr & 0xf000);
> +monitor_printf(mon, "Phy=%08x Flags: ", env->mmu.mmusr & 0xf000);
> /* flags found on the page descriptor */
> if (env->mmu.mmusr & M68K_MMU_G_040) {
> -qemu_printf("G"); /* Global */
> +monitor_puts(mon, "G"); /* Global */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> if (env->mmu.mmusr & M68K_MMU_S_040) {
> -qemu_printf("S"); /* Supervisor */
> +monitor_puts(mon, "S"); /* Supervisor */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> if (env->mmu.mmusr & M68K_MMU_M_040) {
> - qemu_printf("M"); /* Modified */
> +monitor_puts(mon, "M"); /* Modified */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> if (env->mmu.mmusr & M68K_MMU_WP_040) {
> -qemu_printf("W"); /* Write protect */
> +monitor_puts(mon, "W"); /* Write protect */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> if (env->mmu.mmusr & M68K_MMU_T_040) {
> -qemu_printf("T"); /* Transparent */
> +monitor_puts(mon, "T"); /* Transparent */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> if (env->mmu.mmusr & M68K_MMU_R_040) {
> -qemu_printf("R"); /* Resident */
> +monitor_puts(mon, "R"); /* Resident */
> } else {
> -qemu_printf(".");
> +monitor_puts(mon, ".");
> }
> -qemu_printf(" Cache: ");
> +monitor_puts(mon,
Re: [PATCH-for-9.0] monitor/hmp-cmds-target.c: append a space in error message in gpa2hva()
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> On 19/3/24 03:16, Shiyang Ruan via wrote:
> > From: Yao Xingtao
> >
> > In qemu monitor mode, when we use gpa2hva command to print the host
> > virtual address corresponding to a guest physical address, if the gpa is
> > not in RAM, the error message is below:
> >
> > (qemu) gpa2hva 0x75000
> > Memory at address 0x75000is not RAM
> >
> > a space is missed between '0x75000' and 'is'.
> >
> > Signed-off-by: Yao Xingtao
> > ---
> > monitor/hmp-cmds-target.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
> > index 9338ae8440..ff01cf9d8d 100644
> > --- a/monitor/hmp-cmds-target.c
> > +++ b/monitor/hmp-cmds-target.c
> > @@ -261,7 +261,7 @@ void *gpa2hva(MemoryRegion **p_mr, hwaddr addr,
> > uint64_t size, Error **errp)
> > }
> > if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
> > -error_setg(errp, "Memory at address 0x%" HWADDR_PRIx "is not RAM",
> > addr);
> > +error_setg(errp, "Memory at address 0x%" HWADDR_PRIx " is not
> > RAM", addr);
> > memory_region_unref(mrs.mr);
> > return NULL;
> > }
>
> Fixes: e9628441df ("hmp: gpa2hva and gpa2hpa hostaddr command")
> Reviewed-by: Philippe Mathieu-Daudé
Thanks,
Reviewed-by: Dr. David Alan Gilbert
Cc'ing in Trivial.
Dave
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu'
* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Wed, 20 Mar 2024 at 17:06, Philippe Mathieu-Daudé
> wrote:
> >
> > +Alex/Daniel
> >
> > On 20/3/24 17:53, Peter Maydell wrote:
> > > On Wed, 20 Mar 2024 at 16:40, Philippe Mathieu-Daudé
> > > wrote:
> > >>
> > >> 'info tlb' and 'info mem' commands don't scale in heterogeneous
> > >> emulation. They will be reworked after the next release, hidden
> > >> behind the 'info mmu' command. It is not too late to deprecate
> > >> commands, so add the 'info mmu' command as wrapper to the other
> > >> ones, but already deprecate them.
> > >>
> > >> Philippe Mathieu-Daudé (2):
> > >>target/monitor: Introduce 'info mmu' command
> > >>target/monitor: Deprecate 'info tlb' and 'info mem' commands
> > >
> > > This seems to replace "info tlb" and "info mem" with "info mmu -t"
> > > and "info mmu -m", but it doesn't really say anything about:
> > > * what the difference is between these two things
> >
> > I really don't know; I'm only trying to keep the monitor interface
> > identical.
>
> You don't, though: you change it from "info tlb" to "info mmu -t" etc.
>
> > > * which targets implement which and why
> >
> > This one is easy to answer:
> >
> > #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC)
> > || \
> > defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
> > {
> > .name = "tlb",
> >
> > #if defined(TARGET_I386) || defined(TARGET_RISCV)
> > {
> > .name = "mem",
> >
> > > * what the plan is for the future
> >
> > My problem is with linking a single QEMU binary, as these two symbols
> > (hmp_info_mem and hmp_info_tlb) clash.
>
> Yes, but they both (implicitly) operate on the current HMP CPU,
> so the problem with linking into a single binary is that they're
> not indirected through a method on the CPU object, not the syntax
> used in the monitor to invoke them, presumably.
>
> > I'm indeed only postponing the problem, without looking at what
> > this code does. I did it adding hmp_info_mmu_tlb/mem hooks in
> > TCGCPUOps ("hw/core/tcg-cpu-ops.h"), so the command can be
> > dispatched per target vcpu as target-agnostic code in
> > monitor/hmp-cmds.c:
> >
> > +#include "hw/core/tcg-cpu-ops.h"
> > +
> > +static void hmp_info_mmu_tlb(Monitor *mon, CPUState *cpu)
> > +{
> > +const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
> > +
> > +if (tcg_ops->hmp_info_mmu_tlb) {
> > +tcg_ops->hmp_info_mmu_tlb(mon, cpu_env(cpu));
> > +} else {
> > +monitor_puts(mon, "No per-CPU information available on this
> > target\n");
> > +}
> > +}
>
> These aren't TCG specific though, so why TCGCPUOps ?
>
> > > I am definitely not a fan of either of these commands, because
> > > (as we currently implement them) they effectively require each
> > > target architecture to implement a second copy of the page table
> > > walking code. But before we can deprecate them we need to be
> > > pretty sure that "info mmu" is what we want to replace them with.
> >
> > An alternative is to just deprecate them, without adding "info mmu" :)
> >
> > It is OK to un-deprecate stuff if we realize its usefulness.
>
> The commands are there because some users find them useful.
> I just dislike them because I think they're a bit niche and
> annoying to implement and not consistent across target
> architectures and not very well documented...
>
> By the way, we have no obligation to follow the deprecate-and-drop
> process for HMP commands; unlike QMP, we give ourselves the
> license to vary it when we feel like it, because the users are
> humans, not programs or scripts.
Right, so no rush to get the deprecation in; change it when you agree
what you'd like a replacement to look like.
Dave
> -- PMM
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH] hmp: Add option to info qtree to omit details
* BALATON Zoltan (bala...@eik.bme.hu) wrote:
> The output of info qtree monitor command is very long. Add an option
> to print a brief overview omitting all the details.
>
> Signed-off-by: BALATON Zoltan
Seems OK to me (I'm not sure I'd have both 'brief' and 'detailed',
but rather stick to one or the other as variable names); still:
Reviewed-by: Dr. David Alan Gilbert
> ---
> hmp-commands-info.hx | 6 +++---
> system/qdev-monitor.c | 24 +---
> 2 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index da120f82a3..ad1b1306e3 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -540,9 +540,9 @@ ERST
>
> {
> .name = "qtree",
> -.args_type = "",
> -.params = "",
> -.help = "show device tree",
> +.args_type = "brief:-b",
> +.params = "[-b]",
> +.help = "show device tree (-b: brief, omit properties)",
> .cmd= hmp_info_qtree,
> },
>
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index a13db763e5..d5cef36800 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -744,7 +744,6 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> }
>
> #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ##
> __VA_ARGS__)
> -static void qbus_print(Monitor *mon, BusState *bus, int indent);
>
> static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
> int indent)
> @@ -784,13 +783,9 @@ static void bus_print_dev(BusState *bus, Monitor *mon,
> DeviceState *dev, int ind
> static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
> {
> ObjectClass *class;
> -BusState *child;
> NamedGPIOList *ngl;
> NamedClockList *ncl;
>
> -qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
> -dev->id ? dev->id : "");
> -indent += 2;
> QLIST_FOREACH(ngl, >gpios, node) {
> if (ngl->num_in) {
> qdev_printf("gpio-in \"%s\" %d\n", ngl->name ? ngl->name : "",
> @@ -814,12 +809,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev,
> int indent)
> class = object_class_get_parent(class);
> } while (class != object_class_by_name(TYPE_DEVICE));
> bus_print_dev(dev->parent_bus, mon, dev, indent);
> -QLIST_FOREACH(child, >child_bus, sibling) {
> -qbus_print(mon, child, indent);
> -}
> }
>
> -static void qbus_print(Monitor *mon, BusState *bus, int indent)
> +static void qbus_print(Monitor *mon, BusState *bus, int indent, bool details)
> {
> BusChild *kid;
>
> @@ -827,16 +819,26 @@ static void qbus_print(Monitor *mon, BusState *bus, int
> indent)
> indent += 2;
> qdev_printf("type %s\n", object_get_typename(OBJECT(bus)));
> QTAILQ_FOREACH(kid, >children, sibling) {
> +BusState *child_bus;
> DeviceState *dev = kid->child;
> -qdev_print(mon, dev, indent);
> +qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
> +dev->id ? dev->id : "");
> +if (details) {
> +qdev_print(mon, dev, indent + 2);
> +}
> +QLIST_FOREACH(child_bus, >child_bus, sibling) {
> +qbus_print(mon, child_bus, indent + 2, details);
> + }
> }
> }
> #undef qdev_printf
>
> void hmp_info_qtree(Monitor *mon, const QDict *qdict)
> {
> +bool brief = qdict_get_try_bool(qdict, "brief", false);
> +
> if (sysbus_get_default())
> -qbus_print(mon, sysbus_get_default(), 0);
> +qbus_print(mon, sysbus_get_default(), 0, !brief);
> }
>
> void hmp_info_qdm(Monitor *mon, const QDict *qdict)
> --
> 2.30.9
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
core.git: sdext/source
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx | 42 --
1 file changed, 20 insertions(+), 22 deletions(-)
New commits:
commit 1a7f06f01315992b9dfb05e1f9c46307cd72dac8
Author: Dr. David Alan Gilbert
AuthorDate: Mon Mar 4 01:56:56 2024 +
Commit: Noel Grandin
CommitDate: Thu Mar 7 07:04:08 2024 +0100
sdext.pdfimport: Use std::unique_ptr
as per Noel's suggestion - this also ends up fixing some leaks as well.
Change-Id: Ia6099afc1955c341256ec0de5a0f839c005d9b76
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164446
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
index ec2632b74899..603155a2036e 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
@@ -453,8 +453,8 @@ static void flipSplashBitmap(SplashBitmap *pBitmap)
auto nRowSize = static_cast(pBitmap->getRowSize());
auto nAlphaRowSize = static_cast(pBitmap->getAlphaRowSize());
-auto aTmpRow = new unsigned char[nRowSize];
-auto aTmpAlphaRow = new unsigned char[nAlphaRowSize];
+std::unique_ptr aTmpRow(new unsigned char[nRowSize]);
+std::unique_ptr aTmpAlphaRow(new unsigned
char[nAlphaRowSize]);
auto pBitmapData = pBitmap->getDataPtr();
auto pAlphaData = pBitmap->getAlphaPtr();
@@ -470,16 +470,14 @@ static void flipSplashBitmap(SplashBitmap *pBitmap)
nCur++, pCurRowA+=nRowSize, pCurRowB-=nRowSize,
pCurAlphaA+=nAlphaRowSize, pCurAlphaB-=nAlphaRowSize)
{
-memcpy(aTmpRow, pCurRowA, nRowSize);
+memcpy(aTmpRow.get(), pCurRowA, nRowSize);
memcpy(pCurRowA, pCurRowB, nRowSize);
-memcpy(pCurRowB, aTmpRow, nRowSize);
+memcpy(pCurRowB, aTmpRow.get(), nRowSize);
-memcpy(aTmpAlphaRow, pCurAlphaA, nAlphaRowSize);
+memcpy(aTmpAlphaRow.get(), pCurAlphaA, nAlphaRowSize);
memcpy(pCurAlphaA, pCurAlphaB, nAlphaRowSize);
-memcpy(pCurAlphaB, aTmpAlphaRow, nAlphaRowSize);
+memcpy(pCurAlphaB, aTmpAlphaRow.get(), nAlphaRowSize);
}
-delete[] aTmpRow;
-delete[] aTmpAlphaRow;
}
int PDFOutDev::parseFont( long long nNewId, GfxFont* gfxFont, const GfxState*
state ) const
@@ -1268,13 +1266,15 @@ poppler_bool PDFOutDev::tilingPatternFill(GfxState
*state, Gfx *, Catalog *,
auto pSplashGfx = new Gfx(m_pDoc, pSplashOut, pResDict, , nullptr);
pSplashGfx->display(aStr);
-auto pSplashBitmap = pSplashOut->takeBitmap();
+std::unique_ptr pSplashBitmap(pSplashOut->takeBitmap());
+// Poppler tells us to free the splash device immediately after taking the
+// bitmap
delete pSplashGfxState;
delete pSplashGfx;
delete pSplashOut;
// Add a vertical flip, we can't do this in LO for an image filled poly
-flipSplashBitmap(pSplashBitmap);
+flipSplashBitmap(pSplashBitmap.get());
auto nBitmapWidth = static_cast(pSplashBitmap->getWidth());
auto nBitmapHeight = static_cast(pSplashBitmap->getHeight());
@@ -1300,23 +1300,21 @@ poppler_bool PDFOutDev::tilingPatternFill(GfxState
*state, Gfx *, Catalog *,
}
}
-auto pRgbStr = new MemStream(pBitmapData, 0,
-nBitmapWidth * nBitmapHeight * 3, Object(objNull));
-auto pAlphaStr = new MemStream(reinterpret_cast(pSplashBitmap->getAlphaPtr()), 0,
-nBitmapWidth * nBitmapHeight, Object(objNull));
+std::unique_ptr pRgbStr(new MemStream(pBitmapData, 0,
+nBitmapWidth * nBitmapHeight * 3, Object(objNull)));
+std::unique_ptr pAlphaStr(new MemStream(reinterpret_cast(pSplashBitmap->getAlphaPtr()),
+0, nBitmapWidth * nBitmapHeight, Object(objNull)));
auto aDecode = Object(objNull);
-auto pRgbIdentityColorMap = new GfxImageColorMap(8, , new
GfxDeviceRGBColorSpace());
-auto pGrayIdentityColorMap = new GfxImageColorMap(8, , new
GfxDeviceGrayColorSpace());
+std::unique_ptr pRgbIdentityColorMap(new
GfxImageColorMap(8, ,
+new GfxDeviceRGBColorSpace()));
+std::unique_ptr pGrayIdentityColorMap(new
GfxImageColorMap(8, ,
+new GfxDeviceGrayColorSpace()));
OutputBuffer aBuf; initBuf(aBuf);
-writePng_(aBuf, pRgbStr, nBitmapWidth, nBitmapHeight, pRgbIdentityColorMap,
-pAlphaStr, nBitmapWidth, nBitmapHeight, pGrayIdentityColorMap);
+writePng_(aBuf, pRgbStr.get(), nBitmapWidth, nBitmapHeight,
pRgbIdentityColorMap.get(),
+pAlphaStr.get(), nBitmapWidth, nBitmapHeight,
pGrayIdentityColorMap.get());
writeBinaryBuffer(aBuf);
-delete pAlphaStr;
-delete pRgbStr;
-delete pSplashBitmap;
-
// If we return false here we can fall back to the slow path
return true;
}
core.git: sdext/source
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
New commits:
commit cf776a6fa697924deb7df5c0561e12bbd7cda16a
Author: Dr. David Alan Gilbert
AuthorDate: Sun Feb 18 22:01:43 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:28:48 2024 +0100
tdf#113050 sdext.pdfimport: Enable tilingPatternFill
Now we've got everything wired up, turn it on.
Change-Id: I2333e5163493ef0312619f54d2f90ae266fc655b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163575
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
index f38bf423423b..2a8078422def 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
@@ -284,7 +284,7 @@ namespace pdfi
static void setPageNum( int nNumPages );
void setSkipImages ( bool bSkipImages );
#if POPPLER_CHECK_VERSION(21, 3, 0)
-poppler_bool useTilingPatternFill() override { return false; };
+poppler_bool useTilingPatternFill() override { return true; };
poppler_bool tilingPatternFill(GfxState *state, Gfx *gfx, Catalog *cat,
GfxTilingPattern *tPat, const double
*mat,
int x0, int y0, int x1, int y1,
core.git: sdext/source
sdext/source/pdfimport/tree/drawtreevisiting.cxx |2 ++ sdext/source/pdfimport/tree/pdfiprocessor.cxx |7 ++- sdext/source/pdfimport/tree/writertreevisiting.cxx |3 ++- 3 files changed, 10 insertions(+), 2 deletions(-) New commits: commit 81fbaf4bb9ddc385d4452257d731e4097dfed079 Author: Dr. David Alan Gilbert AuthorDate: Thu Feb 15 00:18:01 2024 + Commit: Noel Grandin CommitDate: Thu Feb 29 08:28:31 2024 +0100 tdf#113050 sdext.pdfimport: Set and write TileWidth/Height Set the tile width/height from the step size and write it into the draw:fill-image-width/height properties. Change-Id: I70d69a6d5e77929bd14282731dd68d3bcafa9c1a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163574 Tested-by: Jenkins Reviewed-by: Noel Grandin diff --git a/sdext/source/pdfimport/tree/drawtreevisiting.cxx b/sdext/source/pdfimport/tree/drawtreevisiting.cxx index 2c02adde8874..7897dbae9d08 100644 --- a/sdext/source/pdfimport/tree/drawtreevisiting.cxx +++ b/sdext/source/pdfimport/tree/drawtreevisiting.cxx @@ -826,6 +826,8 @@ void DrawXmlFinalizer::visit( PolyPolyElement& elem, const std::list< std::uniqu aGCProps[ "draw:fill-image-name" ] = m_rStyleContainer.getStyleName( m_rStyleContainer.getStyleId(style)); +aGCProps[ "draw:fill-image-width" ] = unitMMString(convPx2mm(elem.TileWidth)); +aGCProps[ "draw:fill-image-height" ] = unitMMString(convPx2mm(elem.TileHeight)); } diff --git a/sdext/source/pdfimport/tree/pdfiprocessor.cxx b/sdext/source/pdfimport/tree/pdfiprocessor.cxx index 6f4b45f21a40..2483144250b8 100644 --- a/sdext/source/pdfimport/tree/pdfiprocessor.cxx +++ b/sdext/source/pdfimport/tree/pdfiprocessor.cxx @@ -428,12 +428,17 @@ void PDFIProcessor::tilingPatternFill(int nX0, int nY0, int nX1, int nY1, } // TODO: That clipping might shift the fill pattern offsets +double transformedxStep = nxStep * rMat.m00 + nyStep * rMat.m01; +double transformedyStep = nxStep * rMat.m10 + nyStep * rMat.m11; + auto pPolyElement = ElementFactory::createPolyPolyElement( m_pCurElement, getGCId(getCurrentContext()), aB2DPoly, PATH_EOFILL, // Hmm how do I know if this should be EO or not? -nTile, 0, 0 ); +nTile, +transformedxStep * aScale.getX(), +transformedyStep * -aScale.getY()); pPolyElement->updateGeometry(); pPolyElement->ZOrder = m_nNextZOrder++; } diff --git a/sdext/source/pdfimport/tree/writertreevisiting.cxx b/sdext/source/pdfimport/tree/writertreevisiting.cxx index 746fc9e77eb2..9012aaa21038 100644 --- a/sdext/source/pdfimport/tree/writertreevisiting.cxx +++ b/sdext/source/pdfimport/tree/writertreevisiting.cxx @@ -1004,7 +1004,8 @@ void WriterXmlFinalizer::visit( PolyPolyElement& elem, const std::list< std::uni aGCProps[ "draw:fill-image-name" ] = m_rStyleContainer.getStyleName( m_rStyleContainer.getStyleId(style)); - +aGCProps[ "draw:fill-image-width" ] = unitMMString(convPx2mm(elem.TileWidth)); +aGCProps[ "draw:fill-image-height" ] = unitMMString(convPx2mm(elem.TileHeight)); } // TODO(F1): check whether stuff could be emulated by gradient/bitmap/hatch
core.git: sdext/source
sdext/source/pdfimport/inc/genericelements.hxx | 11 ---
sdext/source/pdfimport/tree/genericelements.cxx |8 ++--
sdext/source/pdfimport/tree/pdfiprocessor.cxx |8
3 files changed, 18 insertions(+), 9 deletions(-)
New commits:
commit 784696e47c7f28dac111b95e61f06a9a1f7cdc97
Author: Dr. David Alan Gilbert
AuthorDate: Wed Feb 14 00:25:13 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:28:08 2024 +0100
tdf#113050 sdext.pdfimport: Add TileWidth and TileHeight fields
We need to scale the tiled image that fills our polygon, so add
width and height fields, we'll fill in and process them in subsequent
patches.
Change-Id: Ib066170ccbc0f4a4c971e1d6df72c3f7df14
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163573
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/inc/genericelements.hxx
b/sdext/source/pdfimport/inc/genericelements.hxx
index 4332d4f1372b..705773b9893a 100644
--- a/sdext/source/pdfimport/inc/genericelements.hxx
+++ b/sdext/source/pdfimport/inc/genericelements.hxx
@@ -213,7 +213,8 @@ namespace pdfi
friend class ElementFactory;
PolyPolyElement( Element* pParent, sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
- sal_Int8 nAction, ImageId nFillImage );
+ sal_Int8 nAction, ImageId nFillImage,
+ double nTileWidth, double nTileHeight );
public:
virtual void visitedBy( ElementTreeVisitor&, const std::list<
std::unique_ptr >::const_iterator& rParentIt ) override;
@@ -226,6 +227,8 @@ namespace pdfi
basegfx::B2DPolyPolygon PolyPoly;
sal_Int8Action;
ImageId FillImage;
+double TileWidth;
+double TileHeight;
};
struct ImageElement final : public DrawElement
@@ -300,8 +303,10 @@ namespace pdfi
createPolyPolyElement( Element* pParent,
sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
- sal_Int8 nAction, ImageId nFillImage )
-{ return new PolyPolyElement( pParent, nGCId, rPolyPoly, nAction,
nFillImage ); }
+ sal_Int8 nAction, ImageId nFillImage,
+ double nTileWidth, double nTileHeight )
+{ return new PolyPolyElement( pParent, nGCId, rPolyPoly, nAction,
+ nFillImage, nTileWidth, nTileHeight ); }
static ImageElement* createImageElement( Element* pParent, sal_Int32
nGCId, ImageId nImage )
{ return new ImageElement( pParent, nGCId, nImage ); }
diff --git a/sdext/source/pdfimport/tree/genericelements.cxx
b/sdext/source/pdfimport/tree/genericelements.cxx
index 100e145608aa..aec9c999fcf6 100644
--- a/sdext/source/pdfimport/tree/genericelements.cxx
+++ b/sdext/source/pdfimport/tree/genericelements.cxx
@@ -125,11 +125,15 @@ PolyPolyElement::PolyPolyElement( Element*
pParent,
sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
sal_Int8 nAction,
- ImageIdnFillImage )
+ ImageIdnFillImage,
+ double nTileWidth,
+ double nTileHeight )
: DrawElement( pParent, nGCId ),
PolyPoly( rPolyPoly ),
Action( nAction ),
- FillImage( nFillImage )
+ FillImage( nFillImage ),
+ TileWidth( nTileWidth ),
+ TileHeight( nTileHeight )
{
}
diff --git a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
index 0ef30a176ed2..6f4b45f21a40 100644
--- a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
+++ b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
@@ -433,7 +433,7 @@ void PDFIProcessor::tilingPatternFill(int nX0, int nY0, int
nX1, int nY1,
getGCId(getCurrentContext()),
aB2DPoly,
PATH_EOFILL, // Hmm how do I know if this should be EO or not?
-nTile );
+nTile, 0, 0 );
pPolyElement->updateGeometry();
pPolyElement->ZOrder = m_nNextZOrder++;
}
@@ -448,7 +448,7 @@ void PDFIProcessor::strokePath( const uno::Reference<
rendering::XPolyPolygon2D
getGCId(getCurrentContext()),
aPoly,
PATH_STROKE,
--1 );
+-1, 0, 0 );
pPoly->updateGeometry();
pPoly->ZOrder = m_nNextZOrder++;
}
@@ -463,7 +463,7 @@ void PDFIProcessor::fillPath( const uno::Reference<
rendering::XPolyPol
core.git: sdext/source
sdext/source/pdfimport/tree/drawtreevisiting.cxx | 21 -
sdext/source/pdfimport/tree/writertreevisiting.cxx | 21 -
2 files changed, 40 insertions(+), 2 deletions(-)
New commits:
commit 4b295b1b77b33c9a5b5fcfab58132ca0dcb7f90b
Author: Dr. David Alan Gilbert
AuthorDate: Mon Feb 12 01:24:07 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:27:48 2024 +0100
tdf#113050 sdext.pdfimport: Create the fill-image style and use it
In a poly which is being image filled, we need to create a style
with the actual image definition in the 'Contents'
and then set the (automatically created) name of this in
the prop on the main style.
Also we need to set draw:fill to "bitmap" rather than "solid"
Change-Id: I253704519011e98fd106331ccfb139ad93ef6dee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163572
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/tree/drawtreevisiting.cxx
b/sdext/source/pdfimport/tree/drawtreevisiting.cxx
index 5aae54415800..2c02adde8874 100644
--- a/sdext/source/pdfimport/tree/drawtreevisiting.cxx
+++ b/sdext/source/pdfimport/tree/drawtreevisiting.cxx
@@ -817,10 +817,29 @@ void DrawXmlFinalizer::visit( PolyPolyElement& elem,
const std::list< std::uniqu
aGCProps[ "draw:stroke" ] = "none";
}
+if (elem.FillImage != -1)
+{
+PropertyMap props;
+// The image isn't actually in a prop, it's in an extra chunk inside.
+StyleContainer::Style style("draw:fill-image"_ostr, std::move(props));
+style.Contents =
m_rProcessor.getImages().asBase64EncodedString(elem.FillImage);
+aGCProps[ "draw:fill-image-name" ] =
+m_rStyleContainer.getStyleName(
+m_rStyleContainer.getStyleId(style));
+
+}
+
// TODO(F1): check whether stuff could be emulated by gradient/bitmap/hatch
if( elem.Action & (PATH_FILL | PATH_EOFILL) )
{
-aGCProps[ "draw:fill" ] = "solid";
+if (elem.FillImage == -1)
+{
+aGCProps[ "draw:fill" ] = "solid";
+}
+else
+{
+aGCProps[ "draw:fill" ] = "bitmap";
+}
aGCProps[ "draw:fill-color" ] = getColorString(rGC.FillColor);
if (rGC.FillColor.Alpha != 1.0)
aGCProps["draw:opacity"] = getPercentString(rGC.FillColor.Alpha *
100.0);
diff --git a/sdext/source/pdfimport/tree/writertreevisiting.cxx
b/sdext/source/pdfimport/tree/writertreevisiting.cxx
index 9ecce8f48be4..746fc9e77eb2 100644
--- a/sdext/source/pdfimport/tree/writertreevisiting.cxx
+++ b/sdext/source/pdfimport/tree/writertreevisiting.cxx
@@ -995,10 +995,29 @@ void WriterXmlFinalizer::visit( PolyPolyElement& elem,
const std::list< std::uni
aGCProps[ "draw:stroke" ] = "none";
}
+if (elem.FillImage != -1)
+{
+PropertyMap props;
+// The image isn't actually in a prop, it's in an extra chunk inside.
+StyleContainer::Style style("draw:fill-image"_ostr, std::move(props));
+style.Contents =
m_rProcessor.getImages().asBase64EncodedString(elem.FillImage);
+aGCProps[ "draw:fill-image-name" ] =
+m_rStyleContainer.getStyleName(
+m_rStyleContainer.getStyleId(style));
+
+}
+
// TODO(F1): check whether stuff could be emulated by gradient/bitmap/hatch
if( elem.Action & (PATH_FILL | PATH_EOFILL) )
{
-aGCProps[ "draw:fill" ] = "solid";
+if (elem.FillImage == -1)
+{
+aGCProps[ "draw:fill" ] = "solid";
+}
+else
+{
+aGCProps[ "draw:fill" ] = "bitmap";
+}
aGCProps[ "draw:fill-color" ] = getColorString( rGC.FillColor );
}
else
core.git: sdext/source
sdext/source/pdfimport/inc/pdfiprocessor.hxx |1 +
1 file changed, 1 insertion(+)
New commits:
commit 9f21f816a16914e06ff141a800a63f0966e387b2
Author: Dr. David Alan Gilbert
AuthorDate: Mon Feb 12 01:29:38 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:26:58 2024 +0100
tdf#113050 sdext.pdfimport: Expose the ImageContainer const
I need the finalisers to be able to read an image, they have
a ref to the processor but not the emitter; so allow the container
to be read via the processor reference.
Change-Id: Ifd3b2af1d456561ad42ae3e7c664f03b2e0c971c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163571
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/inc/pdfiprocessor.hxx
b/sdext/source/pdfimport/inc/pdfiprocessor.hxx
index 40cdbfdd48c9..89a09d1780bf 100644
--- a/sdext/source/pdfimport/inc/pdfiprocessor.hxx
+++ b/sdext/source/pdfimport/inc/pdfiprocessor.hxx
@@ -71,6 +71,7 @@ namespace pdfi
const GraphicsContext& getGraphicsContext( sal_Int32 nGCId ) const;
GraphicsContext& getCurrentContext() { return m_aGCStack.back(); }
const GraphicsContext& getCurrentContext() const { return
m_aGCStack.back(); }
+const ImageContainer& getImages() const { return m_aImages; };
const css::uno::Reference< css::task::XStatusIndicator >&
getStatusIndicator() const
{ return m_xStatusIndicator; }
core.git: sdext/source
sdext/source/pdfimport/inc/genericelements.hxx |7 ---
sdext/source/pdfimport/tree/genericelements.cxx |6 --
sdext/source/pdfimport/tree/pdfiprocessor.cxx | 15 ++-
3 files changed, 18 insertions(+), 10 deletions(-)
New commits:
commit 2bf5664823e7ef71d917fe95a2c3d92e46d77c32
Author: Dr. David Alan Gilbert
AuthorDate: Mon Feb 12 01:14:06 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:26:40 2024 +0100
tdf#113050 sdext.pdfimport: Add FillImage field to PolyPolyElement
Use -1 to mean the existing solid fill, otherwise it's the ID
of the image to use as the fill.
Change-Id: I596c26145f5285f75af631a3bb7ddf09600982a6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163570
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/inc/genericelements.hxx
b/sdext/source/pdfimport/inc/genericelements.hxx
index 6d1459a1f032..4332d4f1372b 100644
--- a/sdext/source/pdfimport/inc/genericelements.hxx
+++ b/sdext/source/pdfimport/inc/genericelements.hxx
@@ -213,7 +213,7 @@ namespace pdfi
friend class ElementFactory;
PolyPolyElement( Element* pParent, sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
- sal_Int8 nAction );
+ sal_Int8 nAction, ImageId nFillImage );
public:
virtual void visitedBy( ElementTreeVisitor&, const std::list<
std::unique_ptr >::const_iterator& rParentIt ) override;
@@ -225,6 +225,7 @@ namespace pdfi
basegfx::B2DPolyPolygon PolyPoly;
sal_Int8Action;
+ImageId FillImage;
};
struct ImageElement final : public DrawElement
@@ -299,8 +300,8 @@ namespace pdfi
createPolyPolyElement( Element* pParent,
sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
- sal_Int8 nAction)
-{ return new PolyPolyElement( pParent, nGCId, rPolyPoly, nAction ); }
+ sal_Int8 nAction, ImageId nFillImage )
+{ return new PolyPolyElement( pParent, nGCId, rPolyPoly, nAction,
nFillImage ); }
static ImageElement* createImageElement( Element* pParent, sal_Int32
nGCId, ImageId nImage )
{ return new ImageElement( pParent, nGCId, nImage ); }
diff --git a/sdext/source/pdfimport/tree/genericelements.cxx
b/sdext/source/pdfimport/tree/genericelements.cxx
index 1d11cd0d914e..100e145608aa 100644
--- a/sdext/source/pdfimport/tree/genericelements.cxx
+++ b/sdext/source/pdfimport/tree/genericelements.cxx
@@ -124,10 +124,12 @@ void ImageElement::visitedBy( ElementTreeVisitor&
rVisi
PolyPolyElement::PolyPolyElement( Element* pParent,
sal_Int32 nGCId,
const basegfx::B2DPolyPolygon& rPolyPoly,
- sal_Int8 nAction )
+ sal_Int8 nAction,
+ ImageIdnFillImage )
: DrawElement( pParent, nGCId ),
PolyPoly( rPolyPoly ),
- Action( nAction )
+ Action( nAction ),
+ FillImage( nFillImage )
{
}
diff --git a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
index 32280f8fd110..0ef30a176ed2 100644
--- a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
+++ b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
@@ -389,9 +389,10 @@ void PDFIProcessor::tilingPatternFill(int nX0, int nY0,
int nX1, int nY1,
double nxStep, double nyStep,
int /* nPaintType */,
css::geometry::AffineMatrix2D& rMat,
- const
css::uno::Sequence& /*xTile*/)
+ const
css::uno::Sequence& xTile)
{
const GraphicsContext& rGC(getCurrentContext());
+auto nTile = m_aImages.addImage(xTile);
basegfx::B2DTuple aScale, aTranslation;
double fRotate, fShearX;
@@ -431,7 +432,8 @@ void PDFIProcessor::tilingPatternFill(int nX0, int nY0, int
nX1, int nY1,
m_pCurElement,
getGCId(getCurrentContext()),
aB2DPoly,
-PATH_EOFILL ); // Hmm how do I know if this should be EO or not?
+PATH_EOFILL, // Hmm how do I know if this should be EO or not?
+nTile );
pPolyElement->updateGeometry();
pPolyElement->ZOrder = m_nNextZOrder++;
}
@@ -445,7 +447,8 @@ void PDFIProcessor::strokePath( const uno::Reference<
rendering::XPolyPolygon2D
m_pCurElement,
getGCId(getCurrentContext()),
aPoly,
-PATH_ST
core.git: sdext/source
sdext/source/pdfimport/inc/imagecontainer.hxx |1
sdext/source/pdfimport/tree/imagecontainer.cxx | 31 +
2 files changed, 32 insertions(+)
New commits:
commit d7e5eae44e18ab89e85a0e6ed633853ede70ec71
Author: Dr. David Alan Gilbert
AuthorDate: Mon Feb 12 00:44:31 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:26:04 2024 +0100
tdf#113050 sdext.pdfimport: Add ImageContainer::asBase64EncodedString'
For fill-images we need the image as a string.
Change-Id: I4a8429563b0e19ad977b4e933a0ffee378dab244
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163569
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/inc/imagecontainer.hxx
b/sdext/source/pdfimport/inc/imagecontainer.hxx
index d21ed7504f63..03672e8ca8b2 100644
--- a/sdext/source/pdfimport/inc/imagecontainer.hxx
+++ b/sdext/source/pdfimport/inc/imagecontainer.hxx
@@ -42,6 +42,7 @@ namespace pdfi
ImageId addImage( const css::uno::Sequence<
css::beans::PropertyValue>& xBitmap );
void writeBase64EncodedStream( ImageId nImageId, EmitContext& rContext
);
+OUString asBase64EncodedString( ImageId nId ) const;
};
}
diff --git a/sdext/source/pdfimport/tree/imagecontainer.cxx
b/sdext/source/pdfimport/tree/imagecontainer.cxx
index a7154164d771..b9f7ef52e061 100644
--- a/sdext/source/pdfimport/tree/imagecontainer.cxx
+++ b/sdext/source/pdfimport/tree/imagecontainer.cxx
@@ -141,6 +141,37 @@ void ImageContainer::writeBase64EncodedStream( ImageId
nId, EmitContext& rContex
rContext.rEmitter.write( encodeBase64( aData.getConstArray(),
aData.getLength() ));
}
+OUString ImageContainer::asBase64EncodedString( ImageId nId ) const
+{
+OSL_ASSERT( nId >= 0 && o3tl::make_unsigned(nId) < m_aImages.size() );
+
+const uno::Sequence& rEntry( m_aImages[nId] );
+
+// find "InputSequence" property
+const beans::PropertyValue* pAry(rEntry.getConstArray());
+const sal_Int32 nLen(rEntry.getLength());
+const beans::PropertyValue* pValue(
+std::find_if(pAry, pAry+nLen,
+[] (beans::PropertyValue const& v) -> bool {
+return v.Name == "InputSequence";
+}));
+
+if (pValue == pAry + nLen )
+{
+SAL_WARN("sdext.pdfimport", "InputSequence not found");
+return "";
+}
+
+uno::Sequence aData;
+if( !(pValue->Value >>= aData) )
+{
+SAL_WARN("sdext.pdfimport", "Wrong data type");
+return "";
+}
+
+return encodeBase64( aData.getConstArray(), aData.getLength() );
+}
+
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
core.git: sdext/source
sdext/source/pdfimport/tree/style.cxx |4
1 file changed, 4 insertions(+)
New commits:
commit ff160e93d32c62e09b28393979b3535e01057cdc
Author: Dr. David Alan Gilbert
AuthorDate: Mon Feb 12 00:38:42 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:25:35 2024 +0100
tdf#113050 sdext.pdfimport: In styles wrap 'Contents'
For draw:fill-image we need output like:
Base64stuff
https://gerrit.libreoffice.org/c/core/+/163568
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/tree/style.cxx
b/sdext/source/pdfimport/tree/style.cxx
index b97b1d3e9cdf..969fdc0e9648 100644
--- a/sdext/source/pdfimport/tree/style.cxx
+++ b/sdext/source/pdfimport/tree/style.cxx
@@ -197,7 +197,11 @@ void StyleContainer::impl_emitStyle( sal_Int32
nStyleId,
for(sal_Int32 nSubStyle : rStyle.SubStyles)
impl_emitStyle( nSubStyle, rContext, rContainedElemVisitor );
if( !rStyle.Contents.isEmpty() )
+{
+rContext.rEmitter.beginTag( "office:binary-data", PropertyMap() );
rContext.rEmitter.write( rStyle.Contents );
+rContext.rEmitter.endTag( "office:binary-data" );
+}
if( rStyle.ContainedElement )
rStyle.ContainedElement->visitedBy( rContainedElemVisitor,
std::list>::iterator() );
core.git: sdext/source
sdext/source/pdfimport/tree/style.cxx |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) New commits: commit bc5bd022a9ea8128bd5e9ba02bda48332dccbbe4 Author: Dr. David Alan Gilbert AuthorDate: Sat Feb 10 01:00:18 2024 + Commit: Noel Grandin CommitDate: Thu Feb 29 08:25:08 2024 +0100 tdf#113050 sdext.pdfimport: In styles Specialise draw:fill-image Like stroke-dash, draw:fill-image needs the name setting in the draw:name attribute. Change-Id: Ib9c888765af8bfb0849f0f1ef15f9774808a1661 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163567 Tested-by: Jenkins Reviewed-by: Noel Grandin diff --git a/sdext/source/pdfimport/tree/style.cxx b/sdext/source/pdfimport/tree/style.cxx index 3f1d20eb4244..b97b1d3e9cdf 100644 --- a/sdext/source/pdfimport/tree/style.cxx +++ b/sdext/source/pdfimport/tree/style.cxx @@ -190,7 +190,7 @@ void StyleContainer::impl_emitStyle( sal_Int32 nStyleId, PropertyMap aProps( rStyle.Properties ); if( !rStyle.IsSubStyle ) aProps[ "style:name" ] = getStyleName( nStyleId ); -if (rStyle.Name == "draw:stroke-dash") +if (rStyle.Name == "draw:stroke-dash" || rStyle.Name == "draw:fill-image") aProps[ "draw:name" ] = aProps[ "style:name" ]; rContext.rEmitter.beginTag( rStyle.Name.getStr(), aProps );
core.git: sdext/source
sdext/source/pdfimport/tree/pdfiprocessor.cxx | 52 --
1 file changed, 48 insertions(+), 4 deletions(-)
New commits:
commit 2350357d5cc2ac1787816ce887af6e9f36b8d252
Author: Dr. David Alan Gilbert
AuthorDate: Fri Feb 9 02:42:30 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:24:48 2024 +0100
tdf#113050 sdext.pdfimport: Create poly for tiling pattern
Create a poly for the tiling pattern fill.
Change-Id: Iaeadfe51bed6d4de87f36b3a78145829ea8443e6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163566
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
index cf65223f167a..32280f8fd110 100644
--- a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
+++ b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
@@ -28,6 +28,8 @@
#include
+#include
+
#include
#include
#include
@@ -383,13 +385,55 @@ void PDFIProcessor::drawAlphaMaskedImage(const
uno::Sequence& /*xTile*/)
{
-// TODO
+const GraphicsContext& rGC(getCurrentContext());
+
+basegfx::B2DTuple aScale, aTranslation;
+double fRotate, fShearX;
+auto rTfm = rGC.Transformation;
+rTfm.decompose(aScale, aTranslation, fRotate, fShearX);
+
+// Build a poly covering the whole fill area
+double np0x = nX0 * nxStep;
+double np0y = nY0 * nyStep;
+double np1x = nX1 * nxStep;
+double np1y = nY1 * nyStep;
+
+// Transform with the rMat passed in
+double tmpx, tmpy;
+tmpx = np0x * rMat.m00 + np0y * rMat.m01 + rMat.m02;
+tmpy = np0x * rMat.m10 + np0y * rMat.m11 + rMat.m12;
+np0x = tmpx;
+np0y = tmpy;
+tmpx = np1x * rMat.m00 + np1y * rMat.m01 + rMat.m02;
+tmpy = np1x * rMat.m10 + np1y * rMat.m11 + rMat.m12;
+np1x = tmpx;
+np1y = tmpy;
+
+auto aB2DPoly =
basegfx::B2DPolyPolygon(basegfx::utils::createPolygonFromRect(basegfx::B2DRange(np0x,
np0y, np1x, np1y)));
+aB2DPoly.transform(getCurrentContext().Transformation);
+
+// Clip against current clip path, if any
+basegfx::B2DPolyPolygon aCurClip = getCurrentContext().Clip;
+if( aCurClip.count() ) {
+aB2DPoly = basegfx::utils::clipPolyPolygonOnPolyPolygon( aB2DPoly,
aCurClip,
+ true, /* bInside, keep parts inside the clip */
+ false /* bStroke, filled not stroked */ );
+}
+// TODO: That clipping might shift the fill pattern offsets
+
+auto pPolyElement = ElementFactory::createPolyPolyElement(
+m_pCurElement,
+getGCId(getCurrentContext()),
+aB2DPoly,
+PATH_EOFILL ); // Hmm how do I know if this should be EO or not?
+pPolyElement->updateGeometry();
+pPolyElement->ZOrder = m_nNextZOrder++;
}
void PDFIProcessor::strokePath( const uno::Reference<
rendering::XPolyPolygon2D >& rPath )
core.git: sdext/qa sdext/source
sdext/qa/unit/pdfimport.cxx |8
sdext/source/pdfimport/inc/contentsink.hxx|5 +
sdext/source/pdfimport/inc/pdfiprocessor.hxx |6 ++
sdext/source/pdfimport/tree/pdfiprocessor.cxx |9 +
sdext/source/pdfimport/wrapper/wrapper.cxx| 11 +++
5 files changed, 35 insertions(+), 4 deletions(-)
New commits:
commit 4a13e1890e8a0ed81e8fb17009185fa2b15ebff7
Author: Dr. David Alan Gilbert
AuthorDate: Sun Feb 18 22:06:02 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:24:17 2024 +0100
tdf#113050 sdext.pdfimport: Plumb tiling pattern fill to pdfiprocessor
Change-Id: I68459834fd903d31c9b2e57d637265dd67da4f56
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163565
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/qa/unit/pdfimport.cxx b/sdext/qa/unit/pdfimport.cxx
index 377d08d351f8..18988d6845de 100644
--- a/sdext/qa/unit/pdfimport.cxx
+++ b/sdext/qa/unit/pdfimport.cxx
@@ -428,6 +428,14 @@ namespace
{
}
+virtual void tilingPatternFill(int, int, int, int,
+ double, double,
+ int,
+ css::geometry::AffineMatrix2D&,
+ const
css::uno::Sequence&) override
+{
+}
+
typedef std::unordered_map IdToFontMap;
typedef std::unordered_map
FontToIdMap;
diff --git a/sdext/source/pdfimport/inc/contentsink.hxx
b/sdext/source/pdfimport/inc/contentsink.hxx
index 44f609c3caaa..5354e4370f14 100644
--- a/sdext/source/pdfimport/inc/contentsink.hxx
+++ b/sdext/source/pdfimport/inc/contentsink.hxx
@@ -171,6 +171,11 @@ namespace pdfi
css::beans::PropertyValue>&
xImage,
const css::uno::Sequence<
css::beans::PropertyValue>&
xMask) = 0;
+virtual void tilingPatternFill(int nX0, int nY0, int nX1, int nY1,
+ double nxStep, double nyStep,
+ int nPaintType,
+ css::geometry::AffineMatrix2D& rMat,
+ const
css::uno::Sequence& xTile) = 0;
};
typedef std::shared_ptr ContentSinkSharedPtr;
diff --git a/sdext/source/pdfimport/inc/pdfiprocessor.hxx
b/sdext/source/pdfimport/inc/pdfiprocessor.hxx
index 3fdc146716b3..40cdbfdd48c9 100644
--- a/sdext/source/pdfimport/inc/pdfiprocessor.hxx
+++ b/sdext/source/pdfimport/inc/pdfiprocessor.hxx
@@ -150,6 +150,12 @@ namespace pdfi
const css::uno::Sequence<
css::beans::PropertyValue>&
xMask) override;
+virtual void tilingPatternFill(int nX0, int nY0, int nX1, int nY1,
+ double nxStep, double nyStep,
+ int nPaintType,
+ css::geometry::AffineMatrix2D& rMat,
+ const
css::uno::Sequence& xTile) override;
+
void startIndicator( const OUString& rText );
void endIndicator();
diff --git a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
index 44485f4c2525..cf65223f167a 100644
--- a/sdext/source/pdfimport/tree/pdfiprocessor.cxx
+++ b/sdext/source/pdfimport/tree/pdfiprocessor.cxx
@@ -383,6 +383,15 @@ void PDFIProcessor::drawAlphaMaskedImage(const
uno::Sequence& /*xTile*/)
+{
+// TODO
+}
+
void PDFIProcessor::strokePath( const uno::Reference<
rendering::XPolyPolygon2D >& rPath )
{
basegfx::B2DPolyPolygon
aPoly=basegfx::unotools::b2DPolyPolygonFromXPolyPolygon2D(rPath);
diff --git a/sdext/source/pdfimport/wrapper/wrapper.cxx
b/sdext/source/pdfimport/wrapper/wrapper.cxx
index 6094bfabcbbc..d99103b9867a 100644
--- a/sdext/source/pdfimport/wrapper/wrapper.cxx
+++ b/sdext/source/pdfimport/wrapper/wrapper.cxx
@@ -838,10 +838,13 @@ void LineParser::readTilingPatternFill()
readDouble(aMat.m12);
// The tile is an image with alpha
-[[maybe_unused]]const uno::Sequence aTile (
readImageImpl() );
-(void)aTile; // Unused until later patch
-// TODO
-// use the parsed data
+const uno::Sequence aTile ( readImageImpl() );
+
+m_parser.m_pSink->tilingPatternFill( nX0, nY0, nX1, nY1,
+ nXStep, nYStep,
+ nPaintType,
+ aMat,
+ aTile );
}
void Parser::parseLine( std::string_view aLine )
core.git: sdext/source
sdext/source/pdfimport/wrapper/keyword_list |1
sdext/source/pdfimport/wrapper/wrapper.cxx | 33
2 files changed, 34 insertions(+)
New commits:
commit ec0b9062dc8dba82509183eb865da55827bde4d5
Author: Dr. David Alan Gilbert
AuthorDate: Wed Jan 24 02:10:18 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:23:54 2024 +0100
tdf#113050 sdext.pdfimport Tiling pattern fill parser
Add a parser for the TilingPatternFill entry from the poppler wrapper.
This doesn't get triggered yet until the wrapper enables the
tilingPatternFill.
Change-Id: I34ab84ba1ab9d6773f33d667b324bef5d6c6e409
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163564
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/wrapper/keyword_list
b/sdext/source/pdfimport/wrapper/keyword_list
index bf15ddb2460f..4d1e6d2d4013 100644
--- a/sdext/source/pdfimport/wrapper/keyword_list
+++ b/sdext/source/pdfimport/wrapper/keyword_list
@@ -38,6 +38,7 @@ setTextRenderMode,SETTEXTRENDERMODE
setTransformation,SETTRANSFORMATION
startPage,STARTPAGE
strokePath,STROKEPATH
+tilingPatternFill,TILINGPATTERNFILL
updateBlendMode,UPDATEBLENDMODE
updateCtm,UPDATECTM
updateFillColor,UPDATEFILLCOLOR
diff --git a/sdext/source/pdfimport/wrapper/wrapper.cxx
b/sdext/source/pdfimport/wrapper/wrapper.cxx
index ade4dc5edb6b..6094bfabcbbc 100644
--- a/sdext/source/pdfimport/wrapper/wrapper.cxx
+++ b/sdext/source/pdfimport/wrapper/wrapper.cxx
@@ -107,6 +107,7 @@ enum parseKey {
SETTRANSFORMATION,
STARTPAGE,
STROKEPATH,
+TILINGPATTERNFILL,
UPDATEBLENDMODE,
UPDATECTM,
UPDATEFILLCOLOR,
@@ -195,6 +196,7 @@ public:
void readLink();
void readMaskedImage();
void readSoftMaskedImage();
+void readTilingPatternFill();
};
/** Unescapes line-ending characters in input string. These
@@ -813,6 +815,35 @@ void LineParser::readSoftMaskedImage()
m_parser.m_pSink->drawAlphaMaskedImage( aImage, aMask );
}
+void LineParser::readTilingPatternFill()
+{
+sal_Int32 nX0, nY0, nX1, nY1, nPaintType;
+double nXStep, nYStep;
+geometry::AffineMatrix2D aMat;
+readInt32(nX0);
+readInt32(nY0);
+readInt32(nX1);
+readInt32(nY1);
+
+readDouble(nXStep);
+readDouble(nYStep);
+
+readInt32(nPaintType);
+
+readDouble(aMat.m00);
+readDouble(aMat.m10);
+readDouble(aMat.m01);
+readDouble(aMat.m11);
+readDouble(aMat.m02);
+readDouble(aMat.m12);
+
+// The tile is an image with alpha
+[[maybe_unused]]const uno::Sequence aTile (
readImageImpl() );
+(void)aTile; // Unused until later patch
+// TODO
+// use the parsed data
+}
+
void Parser::parseLine( std::string_view aLine )
{
OSL_PRECOND( m_pSink, "Invalid sink" );
@@ -865,6 +896,8 @@ void Parser::parseLine( std::string_view aLine )
}
case STROKEPATH:
m_pSink->strokePath(lp.readPath()); break;
+case TILINGPATTERNFILL:
+lp.readTilingPatternFill(); break;
case UPDATECTM:
lp.readTransformation(); break;
case UPDATEFILLCOLOR:
core.git: sdext/source
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx | 44 +-
1 file changed, 42 insertions(+), 2 deletions(-)
New commits:
commit b7a63d26466bac7eb7b25233a5a53788bed88c81
Author: Dr. David Alan Gilbert
AuthorDate: Sun Feb 18 22:00:37 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:23:28 2024 +0100
tdf#113050 sdext.pdfimport: Flip bitmap
We need to flip the bitmap between the wrapper and LO, but there's no
easy way to do this in a Poly image fill in LO, so do it as a simple
bitmap flip in the wrapper.
Change-Id: Ifd84d37926c21edf30654d3884be975849a6dca3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163563
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
index 57eb30092141..ec2632b74899 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
@@ -432,8 +432,6 @@ static void writeImage_( OutputBuffer& o_rOutputBuf,
}
// forwarders
-
-
static void writeImageLF( OutputBuffer& o_rOutputBuf,
Stream* str,
int width,
@@ -445,6 +443,45 @@ static void writeMaskLF( OutputBuffer& o_rOutputBuf,
int height,
bool bInvert ) {
writeMask_(o_rOutputBuf,str,width,height,bInvert); }
+// Vertically flip the bitmap
+static void flipSplashBitmap(SplashBitmap *pBitmap)
+{
+if (pBitmap->getRowSize() <= 0)
+return;
+
+auto nBitmapHeight = static_cast(pBitmap->getHeight());
+auto nRowSize = static_cast(pBitmap->getRowSize());
+auto nAlphaRowSize = static_cast(pBitmap->getAlphaRowSize());
+
+auto aTmpRow = new unsigned char[nRowSize];
+auto aTmpAlphaRow = new unsigned char[nAlphaRowSize];
+
+auto pBitmapData = pBitmap->getDataPtr();
+auto pAlphaData = pBitmap->getAlphaPtr();
+
+// Set up pairs of pointers working from each end of the bitmap
+auto pCurRowA = pBitmapData;
+auto pCurAlphaA = pAlphaData;
+auto pCurRowB = pBitmapData+nRowSize*(nBitmapHeight-1);
+auto pCurAlphaB = pAlphaData+nAlphaRowSize*(nBitmapHeight-1);
+
+for (size_t nCur = 0;
+ nCur < nBitmapHeight/2;
+ nCur++, pCurRowA+=nRowSize, pCurRowB-=nRowSize,
+ pCurAlphaA+=nAlphaRowSize, pCurAlphaB-=nAlphaRowSize)
+{
+memcpy(aTmpRow, pCurRowA, nRowSize);
+memcpy(pCurRowA, pCurRowB, nRowSize);
+memcpy(pCurRowB, aTmpRow, nRowSize);
+
+memcpy(aTmpAlphaRow, pCurAlphaA, nAlphaRowSize);
+memcpy(pCurAlphaA, pCurAlphaB, nAlphaRowSize);
+memcpy(pCurAlphaB, aTmpAlphaRow, nAlphaRowSize);
+}
+delete[] aTmpRow;
+delete[] aTmpAlphaRow;
+}
+
int PDFOutDev::parseFont( long long nNewId, GfxFont* gfxFont, const GfxState*
state ) const
{
FontAttributes aNewFont;
@@ -1236,6 +1273,9 @@ poppler_bool PDFOutDev::tilingPatternFill(GfxState
*state, Gfx *, Catalog *,
delete pSplashGfx;
delete pSplashOut;
+// Add a vertical flip, we can't do this in LO for an image filled poly
+flipSplashBitmap(pSplashBitmap);
+
auto nBitmapWidth = static_cast(pSplashBitmap->getWidth());
auto nBitmapHeight = static_cast(pSplashBitmap->getHeight());
core.git: sdext/source
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx | 64 +-
1 file changed, 62 insertions(+), 2 deletions(-)
New commits:
commit 8ac5f40b330c6cd248073b8686c05f566ecd7195
Author: Dr. David Alan Gilbert
AuthorDate: Sun Feb 18 21:45:42 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:22:38 2024 +0100
tdf#113050 sdext.pdfimport: Write the tiling pattern image
Render the tile down to a common format using Splash
and write it to the stream.
Change-Id: Iccc451d1a02ed2ad77538883ab3fe5542fc633e2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163562
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
index e7afe67afef4..57eb30092141 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
@@ -1182,13 +1182,15 @@ void PDFOutDev::setSkipImages( bool bSkipImages )
}
#if POPPLER_CHECK_VERSION(21, 3, 0)
-poppler_bool PDFOutDev::tilingPatternFill(GfxState *, Gfx *, Catalog *,
+poppler_bool PDFOutDev::tilingPatternFill(GfxState *state, Gfx *, Catalog *,
GfxTilingPattern *tPat, const double
*mat,
int x0, int y0, int x1, int y1,
double xStep, double yStep)
{
const double *pBbox = tPat->getBBox();
const int nPaintType = tPat->getPaintType();
+Dict *pResDict = tPat->getResDict();
+Object *aStr = tPat->getContentStream();
double nWidth = pBbox[2] - pBbox[0];
double nHeight = pBbox[3] - pBbox[1];
@@ -1215,7 +1217,65 @@ poppler_bool PDFOutDev::tilingPatternFill(GfxState *,
Gfx *, Catalog *,
normalize(mat[4]), normalize(mat[5])
);
-// TODO: Write the image
+PDFRectangle aBox;
+aBox.x1 = pBbox[0];
+aBox.y1 = pBbox[1];
+aBox.x2 = pBbox[2];
+aBox.y2 = pBbox[3];
+
+const int nDPI = 72; // GfxState seems to have 72.0 as magic for some
reason
+auto pSplashGfxState = new GfxState(nDPI, nDPI, , 0, false);
+auto pSplashOut = new SplashOutputDev(splashModeRGB8, 1, false, nullptr);
+pSplashOut->setEnableFreeType(false);
+pSplashOut->startPage(0 /* pageNum */, pSplashGfxState, nullptr /* xref
*/);
+
+auto pSplashGfx = new Gfx(m_pDoc, pSplashOut, pResDict, , nullptr);
+pSplashGfx->display(aStr);
+auto pSplashBitmap = pSplashOut->takeBitmap();
+delete pSplashGfxState;
+delete pSplashGfx;
+delete pSplashOut;
+
+auto nBitmapWidth = static_cast(pSplashBitmap->getWidth());
+auto nBitmapHeight = static_cast(pSplashBitmap->getHeight());
+
+char *pBitmapData = reinterpret_cast(pSplashBitmap->getDataPtr());
+if (nPaintType == 2)
+{
+// My understanding is Type 2 fills are just bitmaps of *what* to fill
+// in the current fill colour.
+// sending it to LO as a flat colour image with the alpha map is
easiest
+GfxRGB aCurFill;
+unsigned char r,g,b;
+state->getFillColorSpace()->getRGB(state->getFillColor(), );
+r = colToByte(aCurFill.r);
+g = colToByte(aCurFill.g);
+b = colToByte(aCurFill.b);
+
+for(size_t i=0; i < (nBitmapWidth * nBitmapHeight * 3); i+=3)
+{
+pBitmapData[i ] = r;
+pBitmapData[i+1] = g;
+pBitmapData[i+2] = b;
+}
+}
+
+auto pRgbStr = new MemStream(pBitmapData, 0,
+nBitmapWidth * nBitmapHeight * 3, Object(objNull));
+auto pAlphaStr = new MemStream(reinterpret_cast(pSplashBitmap->getAlphaPtr()), 0,
+nBitmapWidth * nBitmapHeight, Object(objNull));
+auto aDecode = Object(objNull);
+auto pRgbIdentityColorMap = new GfxImageColorMap(8, , new
GfxDeviceRGBColorSpace());
+auto pGrayIdentityColorMap = new GfxImageColorMap(8, , new
GfxDeviceGrayColorSpace());
+
+OutputBuffer aBuf; initBuf(aBuf);
+writePng_(aBuf, pRgbStr, nBitmapWidth, nBitmapHeight, pRgbIdentityColorMap,
+pAlphaStr, nBitmapWidth, nBitmapHeight, pGrayIdentityColorMap);
+writeBinaryBuffer(aBuf);
+
+delete pAlphaStr;
+delete pRgbStr;
+delete pSplashBitmap;
// If we return false here we can fall back to the slow path
return true;
core.git: sdext/source
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx | 64 +-
sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx |7 +
2 files changed, 70 insertions(+), 1 deletion(-)
New commits:
commit 8e4a351f12d777ad9102086298741d0a97e5eeb9
Author: Dr. David Alan Gilbert
AuthorDate: Mon Jan 15 02:09:39 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:21:09 2024 +0100
tdf#113050 sdext.pdfimport: Write the tiling pattern header
Add the 'tilingPatternFill' hook to our output device, but don't enable
it yet; we can do that at the end of the series.
Change-Id: Id52b023c3025f5addce1ab09663da9b8bd8a75e4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163561
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
index 2517618e1019..e7afe67afef4 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.cxx
@@ -20,6 +20,25 @@
#include "pdfioutdev_gpl.hxx"
#include "pnghelper.hxx"
+#if defined __GNUC__ || defined __clang__
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wunused-parameter"
+#elif defined _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4100) // unreferenced formal parameter
+#pragma warning(disable : 4121) // alignment of a member was sensitive to
packing in Gfx.h/Operator
+#endif
+
+#include
+#include
+#include
+#if defined __GNUC__ || defined __clang__
+# pragma GCC diagnostic pop
+#elif defined _MSC_VER
+#pragma warning(pop)
+#endif
+
+
#include
#include
#include
@@ -426,7 +445,6 @@ static void writeMaskLF( OutputBuffer& o_rOutputBuf,
int height,
bool bInvert ) {
writeMask_(o_rOutputBuf,str,width,height,bInvert); }
-
int PDFOutDev::parseFont( long long nNewId, GfxFont* gfxFont, const GfxState*
state ) const
{
FontAttributes aNewFont;
@@ -1163,6 +1181,50 @@ void PDFOutDev::setSkipImages( bool bSkipImages )
m_bSkipImages = bSkipImages;
}
+#if POPPLER_CHECK_VERSION(21, 3, 0)
+poppler_bool PDFOutDev::tilingPatternFill(GfxState *, Gfx *, Catalog *,
+ GfxTilingPattern *tPat, const double
*mat,
+ int x0, int y0, int x1, int y1,
+ double xStep, double yStep)
+{
+const double *pBbox = tPat->getBBox();
+const int nPaintType = tPat->getPaintType();
+double nWidth = pBbox[2] - pBbox[0];
+double nHeight = pBbox[3] - pBbox[1];
+
+// If our wrapper is skipping images then we don't need to do anything
+// but return 'true' so that Poppler doesn't do the slow method
+if (m_bSkipImages)
+return true;
+
+// Copied from the Cairo output dev; I think this is patterns
+// with gaps, let poppler do the slow method for now.
+if (xStep != nWidth || yStep != nHeight)
+return false;
+
+printf( "tilingPatternFill %d %d %d %d %f %f "
+"%d "
+"%f %f %f %f %f %f", // No ending space!
+
+x0, y0, x1, y1, normalize(xStep), normalize(yStep),
+
+nPaintType,
+
+normalize(mat[0]), normalize(mat[1]),
+normalize(mat[2]), normalize(mat[3]),
+normalize(mat[4]), normalize(mat[5])
+);
+
+// TODO: Write the image
+
+// If we return false here we can fall back to the slow path
+return true;
+}
+
+// This could be implemented for earlier versions, but the interface keeps
+// changing a little; not having it is only a problem for inputs with
+// large patterns.
+#endif
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
index b34fad1ba90a..f38bf423423b 100644
--- a/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
+++ b/sdext/source/pdfimport/xpdfwrapper/pdfioutdev_gpl.hxx
@@ -283,6 +283,13 @@ namespace pdfi
static void setPageNum( int nNumPages );
void setSkipImages ( bool bSkipImages );
+#if POPPLER_CHECK_VERSION(21, 3, 0)
+poppler_bool useTilingPatternFill() override { return false; };
+poppler_bool tilingPatternFill(GfxState *state, Gfx *gfx, Catalog *cat,
+ GfxTilingPattern *tPat, const double
*mat,
+ int x0, int y0, int x1, int y1,
+ double xStep, double yStep) override;
+#endif
};
}
core.git: external/poppler
external/poppler/StaticLibrary_poppler.mk | 13 +
external/poppler/disable-freetype.patch.1 | 68 ++
external/poppler/poppler-config.patch.1 |2
3 files changed, 82 insertions(+), 1 deletion(-)
New commits:
commit 18a1c3d6c98348d4e3da6e6c9740d6923ab2fba1
Author: Dr. David Alan Gilbert
AuthorDate: Thu Jan 11 01:26:04 2024 +
Commit: Noel Grandin
CommitDate: Thu Feb 29 08:20:20 2024 +0100
tdf#113050 poppler: Enable splash
Enable the splash module, to be used in the xpdf conversion
for tiled patterns.
I've jumped through a couple of hoops to avoid splash
bringing in freetype, extending the existing hack.
This might cause us problems if someone tries to use
text as a tile, but otherwise isOK.
Change-Id: I76ad1268f44137ae5e3205d56170c3a7c55a2dc8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163560
Tested-by: Jenkins
Reviewed-by: Noel Grandin
diff --git a/external/poppler/StaticLibrary_poppler.mk
b/external/poppler/StaticLibrary_poppler.mk
index 7b6f958245ac..726663d983a4 100644
--- a/external/poppler/StaticLibrary_poppler.mk
+++ b/external/poppler/StaticLibrary_poppler.mk
@@ -136,6 +136,7 @@ $(eval $(call
gb_StaticLibrary_add_generated_exception_objects,poppler,\
UnpackedTarball/poppler/poppler/SecurityHandler \
UnpackedTarball/poppler/poppler/SignatureInfo \
UnpackedTarball/poppler/poppler/Sound \
+ UnpackedTarball/poppler/poppler/SplashOutputDev \
UnpackedTarball/poppler/poppler/Stream \
UnpackedTarball/poppler/poppler/StructElement \
UnpackedTarball/poppler/poppler/StructTreeRoot \
@@ -146,7 +147,19 @@ $(eval $(call
gb_StaticLibrary_add_generated_exception_objects,poppler,\
UnpackedTarball/poppler/poppler/UnicodeTypeTable \
UnpackedTarball/poppler/poppler/ViewerPreferences \
UnpackedTarball/poppler/poppler/XRef \
+ UnpackedTarball/poppler/splash/Splash \
UnpackedTarball/poppler/splash/SplashBitmap \
+ UnpackedTarball/poppler/splash/SplashClip \
+ UnpackedTarball/poppler/splash/SplashFont \
+ UnpackedTarball/poppler/splash/SplashFontEngine \
+ UnpackedTarball/poppler/splash/SplashFontFile \
+ UnpackedTarball/poppler/splash/SplashFontFileID \
+ UnpackedTarball/poppler/splash/SplashPath \
+ UnpackedTarball/poppler/splash/SplashPattern \
+ UnpackedTarball/poppler/splash/SplashScreen \
+ UnpackedTarball/poppler/splash/SplashState \
+ UnpackedTarball/poppler/splash/SplashXPath \
+ UnpackedTarball/poppler/splash/SplashXPathScanner \
))
# vim: set noet sw=4 ts=4:
diff --git a/external/poppler/disable-freetype.patch.1
b/external/poppler/disable-freetype.patch.1
index 3c4d1d0f7a8a..d12374f088f9 100644
--- a/external/poppler/disable-freetype.patch.1
+++ b/external/poppler/disable-freetype.patch.1
@@ -39,3 +39,71 @@ disable freetype dependent code
}
std::string Form::getFallbackFontForChar(Unicode uChar, const GfxFont
) const
+--- poppler/splash/SplashFontEngine.cc.orig2024-02-21 01:28:10.479645185
+
popller/splash/SplashFontEngine.cc 2023-09-05 22:15:14.0 +0100
+@@ -66,9 +66,9 @@
+ delete font;
+ }
+
+-if (ftEngine) {
+-delete ftEngine;
+-}
++//if (ftEngine) {
++//delete ftEngine;
++//}
+ }
+
+ SplashFontFile *SplashFontEngine::getFontFile(SplashFontFileID *id)
+--- poppler/splash/SplashFTFontEngine.h.orig 2023-09-05 22:15:14.0
+0100
poppler/splash/SplashFTFontEngine.h2024-02-21 13:53:29.715423742
+
+@@ -26,8 +26,8 @@
+ #ifndef SPLASHFTFONTENGINE_H
+ #define SPLASHFTFONTENGINE_H
+
+-#include
+-#include FT_FREETYPE_H
++//#include
++//#include FT_FREETYPE_H
+
+ class SplashFontFile;
+ class SplashFontFileID;
+@@ -40,7 +40,7 @@
+ class SplashFTFontEngine
+ {
+ public:
+-static SplashFTFontEngine *init(bool aaA, bool enableFreeTypeHintingA,
bool enableSlightHinting);
++static SplashFTFontEngine *init(bool aaA, bool enableFreeTypeHintingA,
bool enableSlightHinting) { return nullptr; };
+
+ ~SplashFTFontEngine();
+
+@@ -48,22 +48,22 @@
+ SplashFTFontEngine =(const SplashFTFontEngine &) = delete;
+
+ // Load fonts.
+-SplashFontFile *loadType1Font(SplashFontFileID *idA, SplashFontSrc *src,
const char **enc);
+-SplashFontFile *loadType1CFont(SplashFontFileID *idA, SplashFontSrc *src,
const char **enc);
+-SplashFontFile *loadOpenTypeT1CFont(SplashFontFileID *idA, SplashFontSrc
*src, const char **enc);
+-SplashFontFile *loadCIDFont(SplashFontFileID *idA, SplashFontSrc *src);
+-SplashFontFile *loadOpenTypeCFFFont(SplashFontFileID *idA, SplashFontSrc
*src, int *codeToGID, int codeToGIDLen);
+-SplashFontFile *loadTrueTypeFont(SplashFontFileID *idA, SplashFontSrc
*src, int *codeToGID, int codeToGIDLen, int faceIndex = 0);
++SplashFontFile *loadType1Font(SplashFontFileID
core.git: sw/CppunitTest_sw_pdf_test.mk
sw/CppunitTest_sw_pdf_test.mk |2 ++ 1 file changed, 2 insertions(+) New commits: commit 56f45bf76f6cd6683e81b9d30131a7a4c5696340 Author: Dr. David Alan Gilbert AuthorDate: Thu Feb 22 11:36:14 2024 + Commit: Mike Kaganski CommitDate: Fri Feb 23 17:02:25 2024 +0100 CppunitTest_sw_pdf_test: Add dependency on xpdfimport The missing dependency would trigger the failure in: ::HybridPdfTest::testNoHybridDataInPDF warn:sal.osl:3173303:3173368:sal/osl/unx/process.cxx:349: Failed to launch child process, child reports ENOENT ? Couldn't exec xpdfimport? Change-Id: I5e887129d2119c71b993d9a3e26b9b17bb912452 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163771 Tested-by: Jenkins Reviewed-by: Mike Kaganski diff --git a/sw/CppunitTest_sw_pdf_test.mk b/sw/CppunitTest_sw_pdf_test.mk index 0d9fd16985e7..6e4126fd3b06 100644 --- a/sw/CppunitTest_sw_pdf_test.mk +++ b/sw/CppunitTest_sw_pdf_test.mk @@ -43,6 +43,8 @@ $(eval $(call gb_CppunitTest_use_externals,sw_pdf_test,\ libxml2 \ )) +$(eval $(call gb_CppunitTest_use_executable,sw_pdf_test,xpdfimport)) + $(eval $(call gb_CppunitTest_set_include,sw_pdf_test,\ -I$(SRCDIR)/sw/inc \ -I$(SRCDIR)/sw/source/core/inc \
Re: Is it still possible to compile LibreOffice 24 for Linux 32-bit? (part 2)
* Escuelas Linux (escuelasli...@gmail.com) wrote: > Thanks to Dan Horák and Rene Engelhard for their answers! Best to keep the reply on the thread! > Dan said: > > > -"Because 32-bit system means max 4GB address space for a process and ld > runs as a single process". > > > I installed a PAE kernel, so it can effectively use more than 4 GB. PAE isn't quite that powerful. It can let the system use more than 4GB total, but any one process can't use more than 2 or 3GB. So it doesn't help if, say your linker process, decides it needs loads of RAM. Fundamentally that's a limit of using 32bit address space. > > -"You can try disabling or reducing the size of debuginfo to reduce the > size of the *.o files if it's used, you can disable LTO and there are some > options for ld to reduce its memory usage a bit". > > > I tried several permutations to reduce the amount of memory used. One of > them was to reduce the number of processors in my autogen.input: > > > --with-parallelism=1 Good. > Another one was to add this parameter when running make: > > > make LDFLAGS="-Wl,--reduce-memory-overheads" That looks like it's a specific trade off; not any of the specific things mentioned about debuginfo and ensuring LTO is disabled. If possible you should probably also include the exact command line of the process that dies. > > I also increased the available memory on the VM from 12 to 15 GB. But no > results, always got the exhausted memory message. Right, see what I said above about PAE not being that smart. > I also tried several other possible solutions, but it feels like going on a > wild goose chase without knowing exactly what to do, relying on some > general random advices found on the web. Dave > > - > > Rene said: > > > -"Debian still ships LibreOffice on 32bit archs, as do other distributions." > > > Oh! Thanks for the tip! I was not aware that Debian even has binary > LibreOffice 24.2 32-bit packages, albeit in the unstable branch. > > I was even able to build LibreOffice from source: > > > apt-get --build source libreoffice > > > This proves to me that the LibreOffice source code has not dropped 32-bit > support, but I still don't know why my own compilation attempts always > failed with a memory exhausted message, even with the same VM configuration. > > Would it be possible for me to just use the Debian LibreOffice 24.2 > binaries or its source packages? Not quite, as they also have a lot of > dependencies on packages that are only available in the unstable branch, so > they would not be suitable for a distribution intended for a stable Debian > 12 bookworm base. > > > -"I do -g1 in Debian. LTO works". > > > Where should I add the -g1 parameter? > > > -"make check fails. I do a minimal set (testtools bridgetest, smoketest, > sal, the other public libraries) to at least not get something > fundamentally broken". > > > My autogen.input file is: > > --with-parallelism=16 > > --with-distro=LibreOfficeLinux > > --with-lang=es > > --with-myspell-dicts > > --enable-release-build > > --with-package-format=deb > > --disable-dependency-tracking > > --with-jdk-home=/usr/lib/jvm/java-11-openjdk-i386/ > > --with-privacy-policy-url=NO-URL > > > -"And somehow the testtools bridgetest fails when building with gcc >= 13, > 12 works. Haven't found a solution yet.)" > > > I'm using gcc 13! Maybe this could be the cause of my compilation failures? > > I'll make some more attempts to compile LibreOffice 32-bit now using gcc > 12, any additional ideas or suggestions would be more than welcome. > > > Thanks again. -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 2/3] hmp: Remove deprecated 'singlestep' command
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> This command has been deprecated before the 8.1 release,
> in commit e9ccfdd91d ("hmp: Add 'one-insn-per-tb' command
> equivalent to 'singlestep'"). Time to drop it.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
> ---
> docs/about/deprecated.rst | 9 -
> docs/about/removed-features.rst | 6 ++
> tests/qtest/test-hmp.c | 1 -
> hmp-commands.hx | 13 -
> 4 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 25527da423..316a26a82c 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -167,15 +167,6 @@ points was removed in 7.0. However QMP still exposed the
> vcpu
> parameter. This argument has now been deprecated and the remaining
> remaining trace points that used it are selected just by name.
>
> -Human Monitor Protocol (HMP) commands
> --
> -
> -``singlestep`` (since 8.1)
> -''
> -
> -The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> -command, which has the same behaviour but a less misleading name.
> -
> Host Architectures
> --
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index a8546f4787..855d788259 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -635,6 +635,12 @@ Use ``migrate-set-parameters`` instead.
>
> This command didn't produce any output already. Removed with no replacement.
>
> +``singlestep`` (removed in 9.0)
> +'''
> +
> +The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> +command, which has the same behaviour but a less misleading name.
> +
> Guest Emulator ISAs
> ---
>
> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
> index fc9125f8bb..1b2e07522f 100644
> --- a/tests/qtest/test-hmp.c
> +++ b/tests/qtest/test-hmp.c
> @@ -64,7 +64,6 @@ static const char *hmp_cmds[] = {
> "qom-get /machine initrd",
> "screendump /dev/null",
> "sendkey x",
> -"singlestep on",
> "wavcapture /dev/null",
> "stopcapture 0",
> "sum 0 512",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 765349ed14..fc5f5c57ad 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -398,19 +398,6 @@ SRST
>If called with option off, the emulation returns to normal mode.
> ERST
>
> -{
> -.name = "singlestep",
> -.args_type = "option:s?",
> -.params = "[on|off]",
> -.help = "deprecated synonym for one-insn-per-tb",
> - .cmd= hmp_one_insn_per_tb,
> -},
> -
> -SRST
> -``singlestep [off]``
> - This is a deprecated synonym for the one-insn-per-tb command.
> -ERST
> -
> {
> .name = "stop|s",
> .args_type = "",
> --
> 2.41.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 2/3] hmp: Remove deprecated 'singlestep' command
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> This command has been deprecated before the 8.1 release,
> in commit e9ccfdd91d ("hmp: Add 'one-insn-per-tb' command
> equivalent to 'singlestep'"). Time to drop it.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
> ---
> docs/about/deprecated.rst | 9 -
> docs/about/removed-features.rst | 6 ++
> tests/qtest/test-hmp.c | 1 -
> hmp-commands.hx | 13 -
> 4 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 25527da423..316a26a82c 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -167,15 +167,6 @@ points was removed in 7.0. However QMP still exposed the
> vcpu
> parameter. This argument has now been deprecated and the remaining
> remaining trace points that used it are selected just by name.
>
> -Human Monitor Protocol (HMP) commands
> --
> -
> -``singlestep`` (since 8.1)
> -''
> -
> -The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> -command, which has the same behaviour but a less misleading name.
> -
> Host Architectures
> --
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index a8546f4787..855d788259 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -635,6 +635,12 @@ Use ``migrate-set-parameters`` instead.
>
> This command didn't produce any output already. Removed with no replacement.
>
> +``singlestep`` (removed in 9.0)
> +'''
> +
> +The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> +command, which has the same behaviour but a less misleading name.
> +
> Guest Emulator ISAs
> ---
>
> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
> index fc9125f8bb..1b2e07522f 100644
> --- a/tests/qtest/test-hmp.c
> +++ b/tests/qtest/test-hmp.c
> @@ -64,7 +64,6 @@ static const char *hmp_cmds[] = {
> "qom-get /machine initrd",
> "screendump /dev/null",
> "sendkey x",
> -"singlestep on",
> "wavcapture /dev/null",
> "stopcapture 0",
> "sum 0 512",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 765349ed14..fc5f5c57ad 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -398,19 +398,6 @@ SRST
>If called with option off, the emulation returns to normal mode.
> ERST
>
> -{
> -.name = "singlestep",
> -.args_type = "option:s?",
> -.params = "[on|off]",
> -.help = "deprecated synonym for one-insn-per-tb",
> - .cmd= hmp_one_insn_per_tb,
> -},
> -
> -SRST
> -``singlestep [off]``
> - This is a deprecated synonym for the one-insn-per-tb command.
> -ERST
> -
> {
> .name = "stop|s",
> .args_type = "",
> --
> 2.41.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 2/3] hmp: Remove deprecated 'singlestep' command
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> This command has been deprecated before the 8.1 release,
> in commit e9ccfdd91d ("hmp: Add 'one-insn-per-tb' command
> equivalent to 'singlestep'"). Time to drop it.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
> ---
> docs/about/deprecated.rst | 9 -
> docs/about/removed-features.rst | 6 ++
> tests/qtest/test-hmp.c | 1 -
> hmp-commands.hx | 13 -
> 4 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 25527da423..316a26a82c 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -167,15 +167,6 @@ points was removed in 7.0. However QMP still exposed the
> vcpu
> parameter. This argument has now been deprecated and the remaining
> remaining trace points that used it are selected just by name.
>
> -Human Monitor Protocol (HMP) commands
> --
> -
> -``singlestep`` (since 8.1)
> -''
> -
> -The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> -command, which has the same behaviour but a less misleading name.
> -
> Host Architectures
> --
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index a8546f4787..855d788259 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -635,6 +635,12 @@ Use ``migrate-set-parameters`` instead.
>
> This command didn't produce any output already. Removed with no replacement.
>
> +``singlestep`` (removed in 9.0)
> +'''
> +
> +The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
> +command, which has the same behaviour but a less misleading name.
> +
> Guest Emulator ISAs
> ---
>
> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
> index fc9125f8bb..1b2e07522f 100644
> --- a/tests/qtest/test-hmp.c
> +++ b/tests/qtest/test-hmp.c
> @@ -64,7 +64,6 @@ static const char *hmp_cmds[] = {
> "qom-get /machine initrd",
> "screendump /dev/null",
> "sendkey x",
> -"singlestep on",
> "wavcapture /dev/null",
> "stopcapture 0",
> "sum 0 512",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 765349ed14..fc5f5c57ad 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -398,19 +398,6 @@ SRST
>If called with option off, the emulation returns to normal mode.
> ERST
>
> -{
> -.name = "singlestep",
> -.args_type = "option:s?",
> -.params = "[on|off]",
> -.help = "deprecated synonym for one-insn-per-tb",
> - .cmd= hmp_one_insn_per_tb,
> -},
> -
> -SRST
> -``singlestep [off]``
> - This is a deprecated synonym for the one-insn-per-tb command.
> -ERST
> -
> {
> .name = "stop|s",
> .args_type = "",
> --
> 2.41.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
core.git: filter/source include/svx svx/source
filter/source/msfilter/escherex.cxx |8
filter/source/msfilter/msdffimp.cxx |9
include/svx/EnhancedCustomShapeGeometry.hxx |3
svx/source/customshapes/EnhancedCustomShapeGeometry.cxx | 376
svx/source/svdraw/svdoashp.cxx |8
5 files changed, 202 insertions(+), 202 deletions(-)
New commits:
commit 80785bb26a1f96d5f5bced7940473d22af00cb60
Author: Dr. David Alan Gilbert
AuthorDate: Mon Jul 3 18:31:26 2023 +0100
Commit: Hossein
CommitDate: Tue Dec 12 10:57:20 2023 +0100
tdf#147021 Use std::span to avoid SAL_N_ELEMENTS in CustomShape
The final set of these, nailing the 'handles'.
Again, most of the changes are mechanical
Change-Id: Ic65cec21db2366bcfa6ac40bcf9041b081c2ed71
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153889
Tested-by: Jenkins
Reviewed-by: Hossein
diff --git a/filter/source/msfilter/escherex.cxx
b/filter/source/msfilter/escherex.cxx
index caf07865941e..2e2e957e6cf0 100644
--- a/filter/source/msfilter/escherex.cxx
+++ b/filter/source/msfilter/escherex.cxx
@@ -2539,13 +2539,13 @@ bool EscherPropertyContainer::IsDefaultObject(
void EscherPropertyContainer::LookForPolarHandles( const MSO_SPT eShapeType,
sal_Int32& nAdjustmentsWhichNeedsToBeConverted )
{
const mso_CustomShape* pDefCustomShape = GetCustomShapeContent( eShapeType
);
-if ( !(pDefCustomShape && pDefCustomShape->nHandles &&
pDefCustomShape->pHandles) )
+if ( !pDefCustomShape || pDefCustomShape->pHandles.empty() )
return;
-sal_Int32 k, nkCount = pDefCustomShape->nHandles;
-const SvxMSDffHandle* pData = pDefCustomShape->pHandles;
-for ( k = 0; k < nkCount; k++, pData++ )
+sal_Int32 k, nkCount = pDefCustomShape->pHandles.size();
+for (k = 0; k < nkCount; k++)
{
+const SvxMSDffHandle* pData = >pHandles[k];
if ( pData->nFlags & SvxMSDffHandleFlags::POLAR )
{
if ( ( pData->nPositionY >= 0x256 ) || ( pData->nPositionY <=
0x107 ) )
diff --git a/filter/source/msfilter/msdffimp.cxx
b/filter/source/msfilter/msdffimp.cxx
index 0c9875cf43af..e0e378f53430 100644
--- a/filter/source/msfilter/msdffimp.cxx
+++ b/filter/source/msfilter/msdffimp.cxx
@@ -2175,12 +2175,13 @@ void
DffPropertyReader::ApplyCustomShapeGeometryAttributes( SvStream& rIn, SfxIt
else
{
const mso_CustomShape* pDefCustomShape = GetCustomShapeContent(
rObjData.eShapeType );
-if ( pDefCustomShape && pDefCustomShape->nHandles &&
pDefCustomShape->pHandles )
+if (pDefCustomShape && !pDefCustomShape->pHandles.empty())
{
-sal_uInt32 i, nCnt = pDefCustomShape->nHandles;
-const SvxMSDffHandle* pData = pDefCustomShape->pHandles;
-for ( i = 0; i < nCnt; i++, pData++ )
+// TODO: This is very similar to
EscherPropertyContainer::LookForPolarHandles
+sal_uInt32 i, nCnt = pDefCustomShape->pHandles.size();
+for (i = 0; i < nCnt; i++)
{
+const SvxMSDffHandle* pData = >pHandles[i];
if ( pData->nFlags & SvxMSDffHandleFlags::POLAR )
{
if ( ( pData->nPositionY >= 0x256 ) || ( pData->nPositionY
<= 0x107 ) )
diff --git a/include/svx/EnhancedCustomShapeGeometry.hxx
b/include/svx/EnhancedCustomShapeGeometry.hxx
index b95501c288c3..626cede2a88d 100644
--- a/include/svx/EnhancedCustomShapeGeometry.hxx
+++ b/include/svx/EnhancedCustomShapeGeometry.hxx
@@ -81,8 +81,7 @@ struct mso_CustomShape
sal_Int32 nXRef;
sal_Int32 nYRef;
std::span pGluePoints;
-SvxMSDffHandle* pHandles;
-sal_uInt32 nHandles;
+std::span pHandles;
};
#define MSO_I | sal_Int32(0x8000)
diff --git a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
index a70b25292e33..7db0ff931ac4 100644
--- a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
+++ b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
@@ -135,7 +135,7 @@ const mso_CustomShape msoArc =
21600, 21600,
MIN_INT32, MIN_INT32,
std::span(),
-const_cast(mso_sptArcHandle), SAL_N_ELEMENTS(
mso_sptArcHandle )// handles
+std::span(mso_sptArcHandle),
};
const SvxMSDffVertPair mso_sptTextSimpleVert[] =
@@ -152,7 +152,7 @@ const mso_CustomShape msoTextSimple =
21600, 21600,
MIN_INT32, MIN_INT32,
std::span(),
-nullptr, 0 // handles
+std::span() // handles
};
const SvxMSDffVertPair mso_sptRectangleVert[] =
@@ -169,7 +169,7 @@ const mso_CustomShape msoRectangle =
21600, 2160
core.git: include/svx svx/source
include/svx/EnhancedCustomShapeGeometry.hxx |3 svx/source/customshapes/EnhancedCustomShapeGeometry.cxx | 376 svx/source/svdraw/svdoashp.cxx | 18 3 files changed, 198 insertions(+), 199 deletions(-) New commits: commit abdb93ce28febd7c4b4e5dad164ee8a394b499b9 Author: Dr. David Alan Gilbert AuthorDate: Mon Jul 3 15:42:09 2023 +0100 Commit: Hossein CommitDate: Sat Dec 9 00:51:23 2023 +0100 tdf#147021 Use std::span to avoid SAL_N_ELEMENTS in CustomShape as for previous set, this time for TextRectangles Again the data is mechanically search/replaced. Change-Id: I836a0c8384d08da6d8c05cfc6ef9f55c1c90fc12 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153885 Tested-by: Jenkins Reviewed-by: Hossein diff --git a/include/svx/EnhancedCustomShapeGeometry.hxx b/include/svx/EnhancedCustomShapeGeometry.hxx index c776daff606a..b95501c288c3 100644 --- a/include/svx/EnhancedCustomShapeGeometry.hxx +++ b/include/svx/EnhancedCustomShapeGeometry.hxx @@ -75,8 +75,7 @@ struct mso_CustomShape sal_uInt32 nElements; std::span pCalculation; sal_Int32* pDefData; -SvxMSDffTextRectangles* pTextRect; -sal_uInt32 nTextRect; +std::span pTextRect; sal_Int32 nCoordWidth; sal_Int32 nCoordHeight; sal_Int32 nXRef; diff --git a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx index 31085c48b9a2..a70b25292e33 100644 --- a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx +++ b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx @@ -131,7 +131,7 @@ const mso_CustomShape msoArc = const_cast(mso_sptArcSegm), sizeof( mso_sptArcSegm ) >> 1, std::span(mso_sptArcCalc), const_cast(mso_sptArcDefault), -nullptr, 0, +std::span(), 21600, 21600, MIN_INT32, MIN_INT32, std::span(), @@ -148,7 +148,7 @@ const mso_CustomShape msoTextSimple = nullptr, 0, std::span(), nullptr, -nullptr, 0, +std::span(), 21600, 21600, MIN_INT32, MIN_INT32, std::span(), @@ -165,7 +165,7 @@ const mso_CustomShape msoRectangle = nullptr, 0, std::span(), nullptr, -nullptr, 0, +std::span(), 21600, 21600, MIN_INT32, MIN_INT32, std::span(), @@ -210,7 +210,7 @@ const mso_CustomShape msoRoundRectangle = const_cast(mso_sptRoundRectangleSegm), sizeof( mso_sptRoundRectangleSegm ) >> 1, std::span(mso_sptRoundRectangleCalc), const_cast(mso_sptDefault3600), -const_cast(mso_sptRoundRectangleTextRect), SAL_N_ELEMENTS( mso_sptRoundRectangleTextRect ), +std::span(mso_sptRoundRectangleTextRect), 21600, 21600, 10800, 10800, std::span(), @@ -235,7 +235,7 @@ const mso_CustomShape msoRightTriangle = nullptr, 0, std::span(), nullptr, -const_cast(mso_sptRightTriangleTextRect), SAL_N_ELEMENTS( mso_sptRightTriangleTextRect ), +std::span(mso_sptRightTriangleTextRect), 21600, 21600, MIN_INT32, MIN_INT32, std::span(mso_sptRightTriangleGluePoints), @@ -264,7 +264,7 @@ const mso_CustomShape msoEllipse = const_cast(mso_sptEllipseSegm), sizeof( mso_sptEllipseSegm ) >> 1, std::span(), nullptr, -const_cast(mso_sptEllipseTextRect), SAL_N_ELEMENTS( mso_sptEllipseTextRect ), +std::span(mso_sptEllipseTextRect), 21600, 21600, MIN_INT32, MIN_INT32, std::span(mso_sptEllipseGluePoints), @@ -315,7 +315,7 @@ const mso_CustomShape msoParallelogram = const_cast(mso_sptParallelogramSegm), sizeof( mso_sptParallelogramSegm ) >> 1, std::span(mso_sptParallelogramCalc), const_cast(mso_sptDefault5400), -const_cast(mso_sptParallelogramTextRect), SAL_N_ELEMENTS( mso_sptParallelogramTextRect ), +std::span(mso_sptParallelogramTextRect), 21600, 21600, MIN_INT32, MIN_INT32, std::span(mso_sptParallelogramGluePoints), @@ -336,7 +336,7 @@ const mso_CustomShape msoDiamond = nullptr, 0, std::span(), nullptr, -const_cast(mso_sptDiamondTextRect), SAL_N_ELEMENTS( mso_sptDiamondTextRect ), +std::span(mso_sptDiamondTextRect), 21600, 21600, MIN_INT32, MIN_INT32, std::span(mso_sptStandardGluePoints), @@ -380,7 +380,7 @@ const mso_CustomShape msoTrapezoid = const_cast(mso_sptTrapezoidSegm), sizeof( mso_sptTrapezoidSegm ) >> 1, std::span(mso_sptTrapezoidCalc), const_cast(mso_sptDefault5400), -const_cast(mso_sptTrapezoidTextRect), SAL_N_ELEMENTS( mso_sptTrapezoidTextRect ), +std::span(mso_sptTrapezoidTextRect), 21600, 21600, MIN_INT32, MIN_INT32, std::span(mso_sptTrapezoidGluePoints), @@ -471,7 +471,7 @@ const mso_CustomShape msoOctagon =
core.git: include/svx svx/source
include/svx/EnhancedCustomShapeGeometry.hxx |3
svx/source/customshapes/EnhancedCustomShapeGeometry.cxx | 377 +++-
svx/source/svdraw/svdoashp.cxx | 22
3 files changed, 202 insertions(+), 200 deletions(-)
New commits:
commit 4a49d7a3a98ccdca52ea0a4475d05235a111ec0a
Author: Dr. David Alan Gilbert
AuthorDate: Mon Jul 3 15:30:04 2023 +0100
Commit: Hossein
CommitDate: Sat Dec 9 00:47:13 2023 +0100
tdf#147021 Use std::span to avoid SAL_N_ELEMENTS in CustomShape
as for previous set, this time for Calculations.
Again the size is mostly mechanical:
In vim load into a:
:+4,+4s/const_cast[(]\([^)]*\)[)],.*/o3tl::span(\1),/
:+4,+4s/nullptr, 0/o3tl::span()
and run
:%g/^const mso_C.*=/@a
(But has had some fixups after conversion from o3tl to std)
Change-Id: I11cdfffdfb61678c173d3136a8e948fd0c224af7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153884
Tested-by: Jenkins
Reviewed-by: Hossein
diff --git a/include/svx/EnhancedCustomShapeGeometry.hxx
b/include/svx/EnhancedCustomShapeGeometry.hxx
index 3191f1e5d14f..c776daff606a 100644
--- a/include/svx/EnhancedCustomShapeGeometry.hxx
+++ b/include/svx/EnhancedCustomShapeGeometry.hxx
@@ -73,8 +73,7 @@ struct mso_CustomShape
std::span pVertices;
sal_uInt16* pElements;
sal_uInt32 nElements;
-SvxMSDffCalculationData*pCalculation;
-sal_uInt32 nCalculation;
+std::span pCalculation;
sal_Int32* pDefData;
SvxMSDffTextRectangles* pTextRect;
sal_uInt32 nTextRect;
diff --git a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
index ad87e34d5da3..31085c48b9a2 100644
--- a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
+++ b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
@@ -17,7 +17,6 @@
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
*/
-
#include
#include
@@ -130,7 +129,7 @@ const mso_CustomShape msoArc =
{
std::span(mso_sptArcVert),
const_cast(mso_sptArcSegm), sizeof( mso_sptArcSegm ) >> 1,
-const_cast(mso_sptArcCalc), SAL_N_ELEMENTS(
mso_sptArcCalc ),
+std::span(mso_sptArcCalc),
const_cast(mso_sptArcDefault),
nullptr, 0,
21600, 21600,
@@ -147,7 +146,7 @@ const mso_CustomShape msoTextSimple =
{
std::span(mso_sptTextSimpleVert),
nullptr, 0,
-nullptr, 0,
+std::span(),
nullptr,
nullptr, 0,
21600, 21600,
@@ -164,7 +163,7 @@ const mso_CustomShape msoRectangle =
{
std::span(mso_sptRectangleVert),
nullptr, 0,
-nullptr, 0,
+std::span(),
nullptr,
nullptr, 0,
21600, 21600,
@@ -209,7 +208,7 @@ const mso_CustomShape msoRoundRectangle =
{
std::span(mso_sptRoundRectangleVert),
const_cast(mso_sptRoundRectangleSegm), sizeof(
mso_sptRoundRectangleSegm ) >> 1,
-const_cast(mso_sptRoundRectangleCalc),
SAL_N_ELEMENTS( mso_sptRoundRectangleCalc ),
+std::span(mso_sptRoundRectangleCalc),
const_cast(mso_sptDefault3600),
const_cast(mso_sptRoundRectangleTextRect),
SAL_N_ELEMENTS( mso_sptRoundRectangleTextRect ),
21600, 21600,
@@ -234,7 +233,7 @@ const mso_CustomShape msoRightTriangle =
{
std::span(mso_sptRightTriangleVert),
nullptr, 0,
-nullptr, 0,
+std::span(),
nullptr,
const_cast(mso_sptRightTriangleTextRect),
SAL_N_ELEMENTS( mso_sptRightTriangleTextRect ),
21600, 21600,
@@ -263,7 +262,7 @@ const mso_CustomShape msoEllipse =
{
std::span(mso_sptEllipseVert),
const_cast(mso_sptEllipseSegm), sizeof( mso_sptEllipseSegm )
>> 1,
-nullptr, 0,
+std::span(),
nullptr,
const_cast(mso_sptEllipseTextRect),
SAL_N_ELEMENTS( mso_sptEllipseTextRect ),
21600, 21600,
@@ -314,7 +313,7 @@ const mso_CustomShape msoParallelogram =
{
std::span(mso_sptParallelogramVert),
const_cast(mso_sptParallelogramSegm), sizeof(
mso_sptParallelogramSegm ) >> 1,
-const_cast(mso_sptParallelogramCalc),
SAL_N_ELEMENTS( mso_sptParallelogramCalc ),
+std::span(mso_sptParallelogramCalc),
const_cast(mso_sptDefault5400),
const_cast(mso_sptParallelogramTextRect),
SAL_N_ELEMENTS( mso_sptParallelogramTextRect ),
21600, 21600,
@@ -335,7 +334,7 @@ const mso_CustomShape msoDiamond =
{
std::span(mso_sptDiamondVert),
nullptr, 0,
-nullptr, 0,
+std::span(),
nullptr,
const_cast(mso_sptDiamondTextRect),
SAL_N_ELEMENTS( mso_sptDiamondTextRect ),
21600, 21600,
@@ -379,7 +378,7 @@ const mso_CustomShape msoTrapezoid =
{
std::span(mso_sptTrapezoidVert),
const_cast(mso_sptTrapezoidSegm), sizeof(
mso_sptTrapezoidSegm ) >> 1,
-const_cast(mso_sptT
core.git: include/svx svx/source
include/svx/EnhancedCustomShapeGeometry.hxx |3 svx/source/customshapes/EnhancedCustomShapeGeometry.cxx | 379 svx/source/svdraw/svdoashp.cxx | 10 3 files changed, 196 insertions(+), 196 deletions(-) New commits: commit 82f30ac55a90a0f0915d4016c760c5c38db087f1 Author: Dr. David Alan Gilbert AuthorDate: Mon Jul 3 15:05:03 2023 +0100 Commit: Hossein CommitDate: Sat Dec 9 00:46:06 2023 +0100 tdf#147021 Use std::span to avoid SAL_N_ELEMENTS in CustomShape as for previous set, this time the GluePoints. Again the size is mostly mechanical: In vim into the A register: :+9,+9s/const_cast[(]\([^)]*\)[)],.*/o3tl::span(\1),/ :+9,+9s/nullptr, 0/o3tl::span() and then: :%g/^const mso_C.*=/@a for the tables. (But has had some fixups since switching from o3tl to std) Change-Id: I966275caf38a6af469d334a0161053159e726f22 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153883 Tested-by: Jenkins Reviewed-by: Hossein diff --git a/include/svx/EnhancedCustomShapeGeometry.hxx b/include/svx/EnhancedCustomShapeGeometry.hxx index db8fe5a5944f..3191f1e5d14f 100644 --- a/include/svx/EnhancedCustomShapeGeometry.hxx +++ b/include/svx/EnhancedCustomShapeGeometry.hxx @@ -82,8 +82,7 @@ struct mso_CustomShape sal_Int32 nCoordHeight; sal_Int32 nXRef; sal_Int32 nYRef; -SvxMSDffVertPair* pGluePoints; -sal_uInt32 nGluePoints; +std::span pGluePoints; SvxMSDffHandle* pHandles; sal_uInt32 nHandles; }; diff --git a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx index 2fa97e30b32d..ad87e34d5da3 100644 --- a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx +++ b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx @@ -17,6 +17,7 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ + #include #include @@ -134,7 +135,7 @@ const mso_CustomShape msoArc = nullptr, 0, 21600, 21600, MIN_INT32, MIN_INT32, -nullptr, 0, +std::span(), const_cast(mso_sptArcHandle), SAL_N_ELEMENTS( mso_sptArcHandle )// handles }; @@ -151,7 +152,7 @@ const mso_CustomShape msoTextSimple = nullptr, 0, 21600, 21600, MIN_INT32, MIN_INT32, -nullptr, 0, +std::span(), nullptr, 0 // handles }; @@ -168,7 +169,7 @@ const mso_CustomShape msoRectangle = nullptr, 0, 21600, 21600, MIN_INT32, MIN_INT32, -nullptr, 0, +std::span(), nullptr, 0 // handles }; @@ -213,7 +214,7 @@ const mso_CustomShape msoRoundRectangle = const_cast(mso_sptRoundRectangleTextRect), SAL_N_ELEMENTS( mso_sptRoundRectangleTextRect ), 21600, 21600, 10800, 10800, -nullptr, 0, +std::span(), const_cast(mso_sptRoundRectangleHandle), SAL_N_ELEMENTS( mso_sptRoundRectangleHandle ) // handles }; @@ -238,7 +239,7 @@ const mso_CustomShape msoRightTriangle = const_cast(mso_sptRightTriangleTextRect), SAL_N_ELEMENTS( mso_sptRightTriangleTextRect ), 21600, 21600, MIN_INT32, MIN_INT32, -const_cast(mso_sptRightTriangleGluePoints), SAL_N_ELEMENTS( mso_sptRightTriangleGluePoints ), +std::span(mso_sptRightTriangleGluePoints), nullptr, 0 // handles }; @@ -267,7 +268,7 @@ const mso_CustomShape msoEllipse = const_cast(mso_sptEllipseTextRect), SAL_N_ELEMENTS( mso_sptEllipseTextRect ), 21600, 21600, MIN_INT32, MIN_INT32, -const_cast(mso_sptEllipseGluePoints), SAL_N_ELEMENTS( mso_sptEllipseGluePoints ), +std::span(mso_sptEllipseGluePoints), nullptr, 0 // handles }; @@ -318,7 +319,7 @@ const mso_CustomShape msoParallelogram = const_cast(mso_sptParallelogramTextRect), SAL_N_ELEMENTS( mso_sptParallelogramTextRect ), 21600, 21600, MIN_INT32, MIN_INT32, -const_cast(mso_sptParallelogramGluePoints), SAL_N_ELEMENTS( mso_sptParallelogramGluePoints ), +std::span(mso_sptParallelogramGluePoints), const_cast(mso_sptParallelogramHandle), SAL_N_ELEMENTS( mso_sptParallelogramHandle )// handles }; @@ -339,7 +340,7 @@ const mso_CustomShape msoDiamond = const_cast(mso_sptDiamondTextRect), SAL_N_ELEMENTS( mso_sptDiamondTextRect ), 21600, 21600, MIN_INT32, MIN_INT32, -const_cast(mso_sptStandardGluePoints), SAL_N_ELEMENTS( mso_sptStandardGluePoints ), +std::span(mso_sptStandardGluePoints), nullptr, 0 // handles }; @@ -383,7 +384,7 @@ const mso_CustomShape msoTrapezoid = const_cast(mso_sptTrapezoidTextRect), SAL_N_ELEMENTS( mso_sptTrapezoidTextRect ), 21600, 21600, MIN_INT32, MIN_INT32, -const_cast(mso_sptTrapezoidGluePoints), SAL_N_ELEMENTS
core.git: include/svx svx/source
include/svx/EnhancedCustomShapeGeometry.hxx |3
svx/source/customshapes/EnhancedCustomShapeGeometry.cxx | 377
svx/source/svdraw/svdoashp.cxx | 10
3 files changed, 195 insertions(+), 195 deletions(-)
New commits:
commit 9924561a094f1b3b2b78700bd1f1638d9608f881
Author: Dr. David Alan Gilbert
AuthorDate: Mon Jul 3 13:16:42 2023 +0100
Commit: Hossein
CommitDate: Fri Dec 8 17:13:53 2023 +0100
tdf#147021 Use std::span to avoid SAL_N_ELEMENTS in CustomShape
EnhancedCustomShapeGeometry has a set of predefined shapes
defined with classic arrays and each is used in a structure
initialisation like:
const mso_CustomShape msoArc =
{
-const_cast(mso_sptArcVert), SAL_N_ELEMENTS(
mso_sptArcVert ),
by using a span we can turn that into:
+std::span(mso_sptArcVert),
There's potential to get rid of some numbered loops later as well.
The patch is large mostly because of mechanical replacement
Loading EnhancedCustomShapeGeometry.cxx into vim then:
into register A put:
:+2,+2s/const_cast[(]\([^)]*\)[)],.*/o3tl::span(\1),/
and do:
:%g/^const mso_C.*=/@a
(since fixed up to std:: )
Change-Id: Ia670b6aa7cc9f662650de2af76ee271a4469f1f8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153882
Tested-by: Jenkins
Reviewed-by: Hossein
diff --git a/include/svx/EnhancedCustomShapeGeometry.hxx
b/include/svx/EnhancedCustomShapeGeometry.hxx
index a52dc4820b6e..db8fe5a5944f 100644
--- a/include/svx/EnhancedCustomShapeGeometry.hxx
+++ b/include/svx/EnhancedCustomShapeGeometry.hxx
@@ -70,8 +70,7 @@ struct SvxMSDffHandle
};
struct mso_CustomShape
{
-SvxMSDffVertPair* pVertices;
-sal_uInt32 nVertices;
+std::span pVertices;
sal_uInt16* pElements;
sal_uInt32 nElements;
SvxMSDffCalculationData*pCalculation;
diff --git a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
index 8e9ee89fb716..2fa97e30b32d 100644
--- a/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
+++ b/svx/source/customshapes/EnhancedCustomShapeGeometry.cxx
@@ -21,6 +21,7 @@
#include
#include
+#include
#include
#include
@@ -126,7 +127,7 @@ const SvxMSDffHandle mso_sptArcHandle[] =
};
const mso_CustomShape msoArc =
{
-const_cast(mso_sptArcVert), SAL_N_ELEMENTS(
mso_sptArcVert ),
+std::span(mso_sptArcVert),
const_cast(mso_sptArcSegm), sizeof( mso_sptArcSegm ) >> 1,
const_cast(mso_sptArcCalc), SAL_N_ELEMENTS(
mso_sptArcCalc ),
const_cast(mso_sptArcDefault),
@@ -143,7 +144,7 @@ const SvxMSDffVertPair mso_sptTextSimpleVert[] =
};
const mso_CustomShape msoTextSimple =
{
-const_cast(mso_sptTextSimpleVert), SAL_N_ELEMENTS(
mso_sptTextSimpleVert ),
+std::span(mso_sptTextSimpleVert),
nullptr, 0,
nullptr, 0,
nullptr,
@@ -160,7 +161,7 @@ const SvxMSDffVertPair mso_sptRectangleVert[] =
};
const mso_CustomShape msoRectangle =
{
-const_cast(mso_sptRectangleVert), SAL_N_ELEMENTS(
mso_sptRectangleVert ),
+std::span(mso_sptRectangleVert),
nullptr, 0,
nullptr, 0,
nullptr,
@@ -205,7 +206,7 @@ const SvxMSDffHandle mso_sptRoundRectangleHandle[] =
};
const mso_CustomShape msoRoundRectangle =
{
-const_cast(mso_sptRoundRectangleVert), SAL_N_ELEMENTS(
mso_sptRoundRectangleVert ),
+std::span(mso_sptRoundRectangleVert),
const_cast(mso_sptRoundRectangleSegm), sizeof(
mso_sptRoundRectangleSegm ) >> 1,
const_cast(mso_sptRoundRectangleCalc),
SAL_N_ELEMENTS( mso_sptRoundRectangleCalc ),
const_cast(mso_sptDefault3600),
@@ -230,7 +231,7 @@ const SvxMSDffVertPair mso_sptRightTriangleGluePoints[] =
};
const mso_CustomShape msoRightTriangle =
{
-const_cast(mso_sptRightTriangleVert), SAL_N_ELEMENTS(
mso_sptRightTriangleVert ),
+std::span(mso_sptRightTriangleVert),
nullptr, 0,
nullptr, 0,
nullptr,
@@ -259,7 +260,7 @@ const SvxMSDffVertPair mso_sptEllipseGluePoints[] =
};
const mso_CustomShape msoEllipse =
{
-const_cast(mso_sptEllipseVert), SAL_N_ELEMENTS(
mso_sptEllipseVert ),
+std::span(mso_sptEllipseVert),
const_cast(mso_sptEllipseSegm), sizeof( mso_sptEllipseSegm )
>> 1,
nullptr, 0,
nullptr,
@@ -310,7 +311,7 @@ const SvxMSDffHandle mso_sptParallelogramHandle[] =
};
const mso_CustomShape msoParallelogram =
{
-const_cast(mso_sptParallelogramVert), SAL_N_ELEMENTS(
mso_sptParallelogramVert ),
+std::span(mso_sptParallelogramVert),
const_cast(mso_sptParallelogramSegm), sizeof(
mso_sptParallelogramSegm ) >> 1,
const_cast(mso_sptParallelogramCalc),
SAL_N_ELEMENTS( mso_sptParallelogramCalc ),
const_cast(mso_sptDefault5400),
@@ -331,7 +332,7 @@ cons
Bug#1029968: And some patches
As well as the fixes in 6.6, we also need this patchup series here: https://lore.kernel.org/linux-media/ZWibhE350L3BTRK8@gallifrey/T/#t These seem to make it pretty nicely. -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Bug#1029968: And some patches
As well as the fixes in 6.6, we also need this patchup series here: https://lore.kernel.org/linux-media/ZWibhE350L3BTRK8@gallifrey/T/#t These seem to make it pretty nicely. -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Bug#1029968: fixed in 6.6
This looks like it's fixed in 6.6, I think they had a major rewrite in there. It's a conversion to vb2 in the series starting with d1846d72587e9241e73a18da14a325b43700013b There are a couple of minor oddities with that (they list the sequence cost the bttv had) but that's relatively minor. Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Bug#1029968: fixed in 6.6
This looks like it's fixed in 6.6, I think they had a major rewrite in there. It's a conversion to vb2 in the series starting with d1846d72587e9241e73a18da14a325b43700013b There are a couple of minor oddities with that (they list the sequence cost the bttv had) but that's relatively minor. Dave -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 2/7] hmp: Improve sync-profile error message
* Markus Armbruster (arm...@redhat.com) wrote:
> Improve
>
> (qemu) sync-profile of
> Error: Invalid parameter 'of'
>
> to
>
> Error: invalid parameter 'of', expecting 'on', 'off', or 'reset'
>
> Signed-off-by: Markus Armbruster
Reviewed-by: Dr. David Alan Gilbert
> ---
> monitor/hmp-cmds.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 6c559b48c8..871898ac46 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -24,7 +24,6 @@
> #include "qapi/qapi-commands-control.h"
> #include "qapi/qapi-commands-misc.h"
> #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qerror.h"
> #include "qemu/cutils.h"
> #include "hw/intc/intc.h"
> #include "qemu/log.h"
> @@ -138,7 +137,8 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict)
> } else {
> Error *err = NULL;
>
> -error_setg(, QERR_INVALID_PARAMETER, op);
> +error_setg(, "invalid parameter '%s',"
> + " expecting 'on', 'off', or 'reset'", op);
> hmp_handle_error(mon, err);
> }
> }
> --
> 2.41.0
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [RFC PATCH] hmp: implement hmp_ram_memory_dump
* Alex Bennée (alex.ben...@linaro.org) wrote:
> While reviewing the tb-stats series I was confused by the different
> between physical and ram address. This implements the RAM dump so I
> can replicate the disassembly of "info tb". Whether or not that is a
> good idea remains to be discussed.
Do you have the definition of what the address space for info tb is?
> Based-on: 20231003183058.1639121-1-richard.hender...@linaro.org
> Signed-off-by: Alex Bennée
> Cc: Richard Henderson
> ---
> include/monitor/hmp-target.h | 1 +
> monitor/hmp-cmds-target.c| 10 ++
> hmp-commands.hx | 19 ++-
> 3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 730507bd65..a3fa7dc089 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -50,6 +50,7 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict);
> void hmp_info_via(Monitor *mon, const QDict *qdict);
> void hmp_memory_dump(Monitor *mon, const QDict *qdict);
> void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict);
> +void hmp_ram_memory_dump(Monitor *mon, const QDict *qdict);
> void hmp_info_registers(Monitor *mon, const QDict *qdict);
> void hmp_gva2gpa(Monitor *mon, const QDict *qdict);
> void hmp_gpa2hva(Monitor *mon, const QDict *qdict);
> diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
> index 476cf68e81..21ce790502 100644
> --- a/monitor/hmp-cmds-target.c
> +++ b/monitor/hmp-cmds-target.c
> @@ -253,6 +253,16 @@ void hmp_physical_memory_dump(Monitor *mon, const QDict
> *qdict)
> memory_dump(mon, count, format, size, addr, MON_DISAS_GPA);
> }
>
> +void hmp_ram_memory_dump(Monitor *mon, const QDict *qdict)
> +{
> +int count = qdict_get_int(qdict, "count");
> +int format = qdict_get_int(qdict, "format");
How does that does match your described '/countformatsize' syntax
you list below?
This just looks like 3 separate parameters; but I guess that's a copy.
> +int size = qdict_get_int(qdict, "size");
> +hwaddr addr = qdict_get_int(qdict, "addr");
> +
> +memory_dump(mon, count, format, size, addr, MON_DISAS_GRA);
OK, so really the only difference is that flag.
> +}
> +
> void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
> {
> Int128 gpa_region_size;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e1d78ab69d..5a9ecefdcb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -471,7 +471,10 @@ ERST
>
> SRST
> ``x/``\ *fmt* *addr*
> - Virtual memory dump starting at *addr*.
> + Virtual memory dump starting at *addr*. See xp/ for physical memory and
> xr/ for ram
> +
> + *fmt* is a format which tells the command how to format the
> + data. Its syntax is: ``/{count}{format}{size}``
> ERST
>
> {
> @@ -530,7 +533,21 @@ SRST
> 0x000b8090: 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720 0x0720
>
> ERST
> +{
> +.name = "xr",
> +.args_type = "fmt:/,addr:l",
> +.params = "/fmt addr",
> +.help = "ram memory dump starting at 'addr'",
> +.cmd= hmp_ram_memory_dump,
> +},
> +
> +SRST
> +``xr /``\ *fmt* *addr*
> + RAM memory dump starting at *addr*. See xp/ for physical and x/ for
> virtual memory
>
> + *fmt* is a format which tells the command how to format the
> + data. Its syntax is: ``/{count}{format}{size}``
> +ERST
> {
> .name = "gpa2hva",
> .args_type = "addr:l",
> --
> 2.39.2
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 4/4] qapi: introduce CONFIG_READ event
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Wed, Oct 18, 2023 at 02:02:08PM +0200, Markus Armbruster wrote: > > Daniel P. Berrangé writes: > > > > > On Wed, Oct 18, 2023 at 06:51:41AM -0400, Michael S. Tsirkin wrote: > > >> On Wed, Oct 18, 2023 at 12:36:10PM +0200, Markus Armbruster wrote: > > >> > > x- seems safer for management tool that doesn't know about > > >> > > "unstable" properties.. > > >> > > > >> > Easy, traditional, and unreliable :) > > >> > > >> > > But on the other hand, changing from x- to no-prefix is already > > >> > > done when the feature is stable, and thouse who use it already > > >> > > use the latest version of interface, so, removing the prefix is > > >> > > just extra work. > > >> > > > >> > Exactly. > > >> > > > >> > > >> I think "x-" is still better for command line use of properties - we > > >> don't have an API to mark things unstable there, do we? > > > > > > Personally I like to see "x-" prefix present *everywhere* there is > > > an unstable feature, and consider the need to rename when declaring > > > it stable to be good thing as it sets an easily identifiable line > > > in the sand and is self-evident to outside observers. > > > > > > The self-documenting nature of the "x-" prefer is what makes it most > > > compelling to me. A patch submission, or command line invokation or > > > an example QMP command, or a bug report, that exhibit an 'x-' prefix > > > are an immediate red flag to anyone who sees them. > > > > Except when it isn't, like in "x-origin". > > > > > If someone sees a QMP comamnd / a typical giant QEMU command line, > > > they are never going to go look at the QAPI schema to check whether > > > any feature used had an 'unstable' marker. The 'unstable' marker > > > might as well not exist in most cases. > > > > > > IOW, having the 'unstable' flag in the QAPI schema is great for machine > > > introspection, but it isn't a substitute for having an 'x-' prefix used > > > for the benefit of humans IMHO. > > > > I'm not sure there's disagreement. Quoting myself: > > > > The "x-" can remind humans "this is unstable" better than a feature > > flag can (for machines, it's the other way round). > > > > CLI and HMP are for humans. We continue to use "x-" there. > > > > QMP is for machines. The feature flag is the sole source of truth. > > Additional use of "x-" is fine, but not required. > > I guess we have different defintions of "for humans" in this context. > I consider QMP data still relevant for humans, because humans are > reviewing patches to libvirt that add usage of QMP features, or > triaging bug reports that include examples of usage, and in both > cases it is pretty relevant to make unstable features stand out to > the human via the x- prefix IMHO. Using x- for events makes sense to me; the semantics of events can be quite subtle; often you don't find out how broken they are until you wire them through libvirt and up the stack; so it's not impossible you might need to change it - but then without the x- the semantics (rather than existence) of the event is carved in stone. Dave > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
Re: [PATCH 4/4] qapi: introduce CONFIG_READ event
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Wed, Oct 18, 2023 at 02:02:08PM +0200, Markus Armbruster wrote: > > Daniel P. Berrangé writes: > > > > > On Wed, Oct 18, 2023 at 06:51:41AM -0400, Michael S. Tsirkin wrote: > > >> On Wed, Oct 18, 2023 at 12:36:10PM +0200, Markus Armbruster wrote: > > >> > > x- seems safer for management tool that doesn't know about > > >> > > "unstable" properties.. > > >> > > > >> > Easy, traditional, and unreliable :) > > >> > > >> > > But on the other hand, changing from x- to no-prefix is already > > >> > > done when the feature is stable, and thouse who use it already > > >> > > use the latest version of interface, so, removing the prefix is > > >> > > just extra work. > > >> > > > >> > Exactly. > > >> > > > >> > > >> I think "x-" is still better for command line use of properties - we > > >> don't have an API to mark things unstable there, do we? > > > > > > Personally I like to see "x-" prefix present *everywhere* there is > > > an unstable feature, and consider the need to rename when declaring > > > it stable to be good thing as it sets an easily identifiable line > > > in the sand and is self-evident to outside observers. > > > > > > The self-documenting nature of the "x-" prefer is what makes it most > > > compelling to me. A patch submission, or command line invokation or > > > an example QMP command, or a bug report, that exhibit an 'x-' prefix > > > are an immediate red flag to anyone who sees them. > > > > Except when it isn't, like in "x-origin". > > > > > If someone sees a QMP comamnd / a typical giant QEMU command line, > > > they are never going to go look at the QAPI schema to check whether > > > any feature used had an 'unstable' marker. The 'unstable' marker > > > might as well not exist in most cases. > > > > > > IOW, having the 'unstable' flag in the QAPI schema is great for machine > > > introspection, but it isn't a substitute for having an 'x-' prefix used > > > for the benefit of humans IMHO. > > > > I'm not sure there's disagreement. Quoting myself: > > > > The "x-" can remind humans "this is unstable" better than a feature > > flag can (for machines, it's the other way round). > > > > CLI and HMP are for humans. We continue to use "x-" there. > > > > QMP is for machines. The feature flag is the sole source of truth. > > Additional use of "x-" is fine, but not required. > > I guess we have different defintions of "for humans" in this context. > I consider QMP data still relevant for humans, because humans are > reviewing patches to libvirt that add usage of QMP features, or > triaging bug reports that include examples of usage, and in both > cases it is pretty relevant to make unstable features stand out to > the human via the x- prefix IMHO. Using x- for events makes sense to me; the semantics of events can be quite subtle; often you don't find out how broken they are until you wire them through libvirt and up the stack; so it's not impossible you might need to change it - but then without the x- the semantics (rather than existence) of the event is carved in stone. Dave > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/
